We introduce a devfreq driver for the MediaTek Cache Coherent Interconnect
(CCI) used by some MediaTek SoCs.
In this driver, we use the passive devfreq driver to get target frequencies
and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek CCI
is supplied by the same regulators with the little core CPUs.
Signed-off-by: Johnson Wang <johnson.wang@mediatek.com>
Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
---
This patch depends on "devfreq-testing"[1].
[1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing
---
drivers/devfreq/Kconfig | 10 +
drivers/devfreq/Makefile | 1 +
drivers/devfreq/mtk-cci-devfreq.c | 474 ++++++++++++++++++++++++++++++
3 files changed, 485 insertions(+)
create mode 100644 drivers/devfreq/mtk-cci-devfreq.c
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 87eb2b837e68..9754d8b31621 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ
It reads ACTMON counters of memory controllers and adjusts the
operating frequencies and voltages with OPP support.
+config ARM_MEDIATEK_CCI_DEVFREQ
+ tristate "MEDIATEK CCI DEVFREQ Driver"
+ depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST
+ select DEVFREQ_GOV_PASSIVE
+ help
+ This adds a devfreq driver for MediaTek Cache Coherent Interconnect
+ which is shared the same regulators with the cpu cluster. It can track
+ buck voltages and update a proper CCI frequency. Use the notification
+ to get the regulator status.
+
config ARM_RK3399_DMC_DEVFREQ
tristate "ARM RK3399 DMC DEVFREQ Driver"
depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 0b6be92a25d9..bf40d04928d0 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o
obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o
+obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o
obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o
obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk-cci-devfreq.c
new file mode 100644
index 000000000000..b3e31c45a57c
--- /dev/null
+++ b/drivers/devfreq/mtk-cci-devfreq.c
@@ -0,0 +1,474 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 MediaTek Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/devfreq.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
+
+struct mtk_ccifreq_platform_data {
+ int min_volt_shift;
+ int max_volt_shift;
+ int proc_max_volt;
+ int sram_min_volt;
+ int sram_max_volt;
+};
+
+struct mtk_ccifreq_drv {
+ struct device *dev;
+ struct devfreq *devfreq;
+ struct regulator *proc_reg;
+ struct regulator *sram_reg;
+ struct clk *cci_clk;
+ struct clk *inter_clk;
+ int inter_voltage;
+ int pre_voltage;
+ unsigned long pre_freq;
+ /* Avoid race condition for regulators between notify and policy */
+ struct mutex reg_lock;
+ struct notifier_block opp_nb;
+ const struct mtk_ccifreq_platform_data *soc_data;
+ int vtrack_max;
+};
+
+static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int new_voltage)
+{
+ const struct mtk_ccifreq_platform_data *soc_data = drv->soc_data;
+ struct device *dev = drv->dev;
+ int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret;
+ int retry_max = drv->vtrack_max;
+
+ if (!drv->sram_reg) {
+ ret = regulator_set_voltage(drv->proc_reg, new_voltage,
+ drv->soc_data->proc_max_volt);
+ goto out_set_voltage;
+ }
+
+ pre_voltage = regulator_get_voltage(drv->proc_reg);
+ if (pre_voltage < 0) {
+ dev_err(dev, "invalid vproc value: %d\n", pre_voltage);
+ return pre_voltage;
+ }
+
+ pre_vsram = regulator_get_voltage(drv->sram_reg);
+ if (pre_vsram < 0) {
+ dev_err(dev, "invalid vsram value: %d\n", pre_vsram);
+ return pre_vsram;
+ }
+
+ new_vsram = clamp(new_voltage + soc_data->min_volt_shift,
+ soc_data->sram_min_volt, soc_data->sram_max_volt);
+
+ do {
+ if (pre_voltage <= new_voltage) {
+ vsram = clamp(pre_voltage + soc_data->max_volt_shift,
+ soc_data->sram_min_volt, new_vsram);
+ ret = regulator_set_voltage(drv->sram_reg, vsram,
+ soc_data->sram_max_volt);
+ if (ret)
+ return ret;
+
+ if (vsram == soc_data->sram_max_volt ||
+ new_vsram == soc_data->sram_min_volt)
+ voltage = new_voltage;
+ else
+ voltage = vsram - soc_data->min_volt_shift;
+
+ ret = regulator_set_voltage(drv->proc_reg, voltage,
+ soc_data->proc_max_volt);
+ if (ret) {
+ regulator_set_voltage(drv->sram_reg, pre_vsram,
+ soc_data->sram_max_volt);
+ return ret;
+ }
+ } else if (pre_voltage > new_voltage) {
+ voltage = max(new_voltage,
+ pre_vsram - soc_data->max_volt_shift);
+ ret = regulator_set_voltage(drv->proc_reg, voltage,
+ soc_data->proc_max_volt);
+ if (ret)
+ return ret;
+
+ if (voltage == new_voltage)
+ vsram = new_vsram;
+ else
+ vsram = max(new_vsram,
+ voltage + soc_data->min_volt_shift);
+
+ ret = regulator_set_voltage(drv->sram_reg, vsram,
+ soc_data->sram_max_volt);
+ if (ret) {
+ regulator_set_voltage(drv->proc_reg, pre_voltage,
+ soc_data->proc_max_volt);
+ return ret;
+ }
+ }
+
+ pre_voltage = voltage;
+ pre_vsram = vsram;
+
+ if (--retry_max < 0) {
+ dev_err(dev,
+ "over loop count, failed to set voltage\n");
+ return -EINVAL;
+ }
+ } while (voltage != new_voltage || vsram != new_vsram);
+
+out_set_voltage:
+ if (!ret)
+ drv->pre_voltage = new_voltage;
+
+ return ret;
+}
+
+static int mtk_ccifreq_target(struct device *dev, unsigned long *freq,
+ u32 flags)
+{
+ struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev);
+ struct clk *cci_pll = clk_get_parent(drv->cci_clk);
+ struct dev_pm_opp *opp;
+ unsigned long opp_rate;
+ int voltage, pre_voltage, inter_voltage, target_voltage, ret;
+
+ if (!drv)
+ return -EINVAL;
+
+ if (drv->pre_freq == *freq)
+ return 0;
+
+ inter_voltage = drv->inter_voltage;
+
+ opp_rate = *freq;
+ opp = devfreq_recommended_opp(dev, &opp_rate, 1);
+ if (IS_ERR(opp)) {
+ dev_err(dev, "failed to find opp for freq: %ld\n", opp_rate);
+ return PTR_ERR(opp);
+ }
+
+ mutex_lock(&drv->reg_lock);
+
+ voltage = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+
+ if (unlikely(drv->pre_voltage <= 0))
+ pre_voltage = regulator_get_voltage(drv->proc_reg);
+ else
+ pre_voltage = drv->pre_voltage;
+
+ if (pre_voltage < 0) {
+ dev_err(dev, "invalid vproc value: %d\n", pre_voltage);
+ return pre_voltage;
+ }
+
+ /* scale up: set voltage first then freq. */
+ target_voltage = max(inter_voltage, voltage);
+ if (pre_voltage <= target_voltage) {
+ ret = mtk_ccifreq_set_voltage(drv, target_voltage);
+ if (ret) {
+ dev_err(dev, "failed to scale up voltage\n");
+ goto out_restore_voltage;
+ }
+ }
+
+ /* switch the cci clock to intermediate clock source. */
+ ret = clk_set_parent(drv->cci_clk, drv->inter_clk);
+ if (ret) {
+ dev_err(dev, "failed to re-parent cci clock\n");
+ goto out_restore_voltage;
+ }
+
+ /* set the original clock to target rate. */
+ ret = clk_set_rate(cci_pll, *freq);
+ if (ret) {
+ dev_err(dev, "failed to set cci pll rate: %d\n", ret);
+ clk_set_parent(drv->cci_clk, cci_pll);
+ goto out_restore_voltage;
+ }
+
+ /* switch the cci clock back to the original clock source. */
+ ret = clk_set_parent(drv->cci_clk, cci_pll);
+ if (ret) {
+ dev_err(dev, "failed to re-parent cci clock\n");
+ mtk_ccifreq_set_voltage(drv, inter_voltage);
+ goto out_unlock;
+ }
+
+ /*
+ * If the new voltage is lower than the intermediate voltage or the
+ * original voltage, scale down to the new voltage.
+ */
+ if (voltage < inter_voltage || voltage < pre_voltage) {
+ ret = mtk_ccifreq_set_voltage(drv, voltage);
+ if (ret) {
+ dev_err(dev, "failed to scale down voltage\n");
+ goto out_unlock;
+ }
+ }
+
+ drv->pre_freq = *freq;
+ mutex_unlock(&drv->reg_lock);
+
+ return 0;
+
+out_restore_voltage:
+ mtk_ccifreq_set_voltage(drv, pre_voltage);
+
+out_unlock:
+ mutex_unlock(&drv->reg_lock);
+ return ret;
+}
+
+static int mtk_ccifreq_opp_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct dev_pm_opp *opp = data;
+ struct mtk_ccifreq_drv *drv;
+ unsigned long freq, volt;
+
+ drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb);
+
+ if (event == OPP_EVENT_ADJUST_VOLTAGE) {
+ freq = dev_pm_opp_get_freq(opp);
+
+ mutex_lock(&drv->reg_lock);
+ /* current opp item is changed */
+ if (freq == drv->pre_freq) {
+ volt = dev_pm_opp_get_voltage(opp);
+ mtk_ccifreq_set_voltage(drv, volt);
+ }
+ mutex_unlock(&drv->reg_lock);
+ }
+
+ return 0;
+}
+
+static struct devfreq_dev_profile mtk_ccifreq_profile = {
+ .target = mtk_ccifreq_target,
+};
+
+static int mtk_ccifreq_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_ccifreq_drv *drv;
+ struct devfreq_passive_data *passive_data;
+ struct dev_pm_opp *opp;
+ unsigned long rate, opp_volt;
+ int ret;
+
+ drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+ if (!drv)
+ return -ENOMEM;
+
+ drv->dev = dev;
+ drv->soc_data = (const struct mtk_ccifreq_platform_data *)
+ of_device_get_match_data(&pdev->dev);
+ mutex_init(&drv->reg_lock);
+ platform_set_drvdata(pdev, drv);
+
+ drv->cci_clk = devm_clk_get(dev, "cci");
+ if (IS_ERR(drv->cci_clk)) {
+ ret = PTR_ERR(drv->cci_clk);
+ return dev_err_probe(dev, ret,
+ "failed to get cci clk: %d\n", ret);
+ }
+
+ drv->inter_clk = devm_clk_get(dev, "intermediate");
+ if (IS_ERR(drv->inter_clk)) {
+ ret = PTR_ERR(drv->inter_clk);
+ dev_err_probe(dev, ret,
+ "failed to get intermediate clk: %d\n", ret);
+ goto out_free_resources;
+ }
+
+ drv->proc_reg = devm_regulator_get_optional(dev, "proc");
+ if (IS_ERR(drv->proc_reg)) {
+ ret = PTR_ERR(drv->proc_reg);
+ dev_err_probe(dev, ret,
+ "failed to get proc regulator: %d\n", ret);
+ goto out_free_resources;
+ }
+
+ ret = regulator_enable(drv->proc_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable proc regulator\n");
+ goto out_free_resources;
+ }
+
+ drv->sram_reg = regulator_get_optional(dev, "sram");
+ if (IS_ERR(drv->sram_reg))
+ drv->sram_reg = NULL;
+ else {
+ ret = regulator_enable(drv->sram_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable sram regulator\n");
+ goto out_free_resources;
+ }
+ }
+
+ /*
+ * We assume min voltage is 0 and tracking target voltage using
+ * min_volt_shift for each iteration.
+ * The retry_max is 3 times of expeted iteration count.
+ */
+ drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt,
+ drv->soc_data->proc_max_volt),
+ drv->soc_data->min_volt_shift);
+
+ ret = clk_prepare_enable(drv->cci_clk);
+ if (ret)
+ goto out_free_resources;
+
+ ret = clk_prepare_enable(drv->inter_clk);
+ if (ret)
+ goto out_disable_cci_clk;
+
+ ret = dev_pm_opp_of_add_table(dev);
+ if (ret) {
+ dev_err(dev, "failed to add opp table: %d\n", ret);
+ goto out_disable_inter_clk;
+ }
+
+ rate = clk_get_rate(drv->inter_clk);
+ opp = dev_pm_opp_find_freq_ceil(dev, &rate);
+ if (IS_ERR(opp)) {
+ ret = PTR_ERR(opp);
+ dev_err(dev, "failed to get intermediate opp: %d\n", ret);
+ goto out_remove_opp_table;
+ }
+ drv->inter_voltage = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+
+ rate = U32_MAX;
+ opp = dev_pm_opp_find_freq_floor(drv->dev, &rate);
+ if (IS_ERR(opp)) {
+ dev_err(dev, "failed to get opp\n");
+ ret = PTR_ERR(opp);
+ goto out_remove_opp_table;
+ }
+
+ opp_volt = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+ ret = mtk_ccifreq_set_voltage(drv, opp_volt);
+ if (ret) {
+ dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n",
+ opp_volt);
+ goto out_remove_opp_table;
+ }
+
+ passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data),
+ GFP_KERNEL);
+ if (!passive_data) {
+ ret = -ENOMEM;
+ goto out_remove_opp_table;
+ }
+
+ passive_data->parent_type = CPUFREQ_PARENT_DEV;
+ drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile,
+ DEVFREQ_GOV_PASSIVE,
+ passive_data);
+ if (IS_ERR(drv->devfreq)) {
+ ret = -EPROBE_DEFER;
+ dev_err(dev, "failed to add devfreq device: %d\n",
+ PTR_ERR(drv->devfreq));
+ goto out_remove_opp_table;
+ }
+
+ drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier;
+ ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb);
+ if (ret) {
+ dev_err(dev, "failed to register opp notifier: %d\n", ret);
+ goto out_remove_devfreq_device;
+ }
+ return 0;
+
+out_remove_devfreq_device:
+ devm_devfreq_remove_device(dev, drv->devfreq);
+
+out_remove_opp_table:
+ dev_pm_opp_of_remove_table(dev);
+
+out_disable_inter_clk:
+ clk_disable_unprepare(drv->inter_clk);
+
+out_disable_cci_clk:
+ clk_disable_unprepare(drv->cci_clk);
+
+out_free_resources:
+ if (regulator_is_enabled(drv->proc_reg))
+ regulator_disable(drv->proc_reg);
+ if (drv->sram_reg && regulator_is_enabled(drv->sram_reg))
+ regulator_disable(drv->sram_reg);
+
+ if (!IS_ERR(drv->proc_reg))
+ regulator_put(drv->proc_reg);
+ if (!IS_ERR(drv->sram_reg))
+ regulator_put(drv->sram_reg);
+ if (!IS_ERR(drv->cci_clk))
+ clk_put(drv->cci_clk);
+ if (!IS_ERR(drv->inter_clk))
+ clk_put(drv->inter_clk);
+
+ return ret;
+}
+
+static int mtk_ccifreq_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_ccifreq_drv *drv;
+
+ drv = platform_get_drvdata(pdev);
+
+ dev_pm_opp_unregister_notifier(dev, &drv->opp_nb);
+ dev_pm_opp_of_remove_table(dev);
+ clk_disable_unprepare(drv->inter_clk);
+ clk_disable_unprepare(drv->cci_clk);
+ regulator_disable(drv->proc_reg);
+ if (drv->sram_reg)
+ regulator_disable(drv->sram_reg);
+
+ return 0;
+}
+
+static const struct mtk_ccifreq_platform_data mt8183_platform_data = {
+ .min_volt_shift = 100000,
+ .max_volt_shift = 200000,
+ .proc_max_volt = 1150000,
+ .sram_min_volt = 0,
+ .sram_max_volt = 1150000,
+};
+
+static const struct mtk_ccifreq_platform_data mt8186_platform_data = {
+ .min_volt_shift = 100000,
+ .max_volt_shift = 250000,
+ .proc_max_volt = 1118750,
+ .sram_min_volt = 850000,
+ .sram_max_volt = 1118750,
+};
+
+static const struct of_device_id mtk_ccifreq_machines[] = {
+ { .compatible = "mediatek,mt8183-cci", .data = &mt8183_platform_data },
+ { .compatible = "mediatek,mt8186-cci", .data = &mt8186_platform_data },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines);
+
+static struct platform_driver mtk_ccifreq_platdrv = {
+ .probe = mtk_ccifreq_probe,
+ .remove = mtk_ccifreq_remove,
+ .driver = {
+ .name = "mtk-ccifreq",
+ .of_match_table = mtk_ccifreq_machines,
+ },
+};
+module_platform_driver(mtk_ccifreq_platdrv);
+
+MODULE_DESCRIPTION("MediaTek CCI devfreq driver");
+MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
Hi Johnson, On 22. 4. 25. 21:55, Johnson Wang wrote: > We introduce a devfreq driver for the MediaTek Cache Coherent Interconnect > (CCI) used by some MediaTek SoCs. > > In this driver, we use the passive devfreq driver to get target frequencies > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek CCI > is supplied by the same regulators with the little core CPUs. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > This patch depends on "devfreq-testing"[1]. > [1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > --- > drivers/devfreq/Kconfig | 10 + > drivers/devfreq/Makefile | 1 + > drivers/devfreq/mtk-cci-devfreq.c | 474 ++++++++++++++++++++++++++++++ > 3 files changed, 485 insertions(+) > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c Acked-by: Chanwoo Choi <cw00.choi@samsung.com> I updated the passive governor on devfreq-testing[1] branch [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing Could you please test your patches based on devfreq-testing[1] branch? -- Best Regards, Samsung Electronics Chanwoo Choi
On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > We introduce a devfreq driver for the MediaTek Cache Coherent > Interconnect > (CCI) used by some MediaTek SoCs. > > In this driver, we use the passive devfreq driver to get target > frequencies > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek > CCI > is supplied by the same regulators with the little core CPUs. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > This patch depends on "devfreq-testing"[1]. > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > --- > drivers/devfreq/Kconfig | 10 + > drivers/devfreq/Makefile | 1 + > drivers/devfreq/mtk-cci-devfreq.c | 474 > ++++++++++++++++++++++++++++++ > 3 files changed, 485 insertions(+) > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > index 87eb2b837e68..9754d8b31621 100644 > --- a/drivers/devfreq/Kconfig > +++ b/drivers/devfreq/Kconfig > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > It reads ACTMON counters of memory controllers and adjusts > the > operating frequencies and voltages with OPP support. > > +config ARM_MEDIATEK_CCI_DEVFREQ > + tristate "MEDIATEK CCI DEVFREQ Driver" > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > + select DEVFREQ_GOV_PASSIVE > + help > + This adds a devfreq driver for MediaTek Cache Coherent > Interconnect > + which is shared the same regulators with the cpu cluster. It > can track > + buck voltages and update a proper CCI frequency. Use the > notification > + to get the regulator status. > + > config ARM_RK3399_DMC_DEVFREQ > tristate "ARM RK3399 DMC DEVFREQ Driver" > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > index 0b6be92a25d9..bf40d04928d0 100644 > --- a/drivers/devfreq/Makefile > +++ b/drivers/devfreq/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > governor_passive.o > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk- > cci-devfreq.c > new file mode 100644 > index 000000000000..b3e31c45a57c > --- /dev/null > +++ b/drivers/devfreq/mtk-cci-devfreq.c > @@ -0,0 +1,474 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 MediaTek Inc. > + */ > + > +#include <linux/clk.h> > +#include <linux/devfreq.h> > +#include <linux/minmax.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_opp.h> > +#include <linux/regulator/consumer.h> > + > +struct mtk_ccifreq_platform_data { > + int min_volt_shift; > + int max_volt_shift; > + int proc_max_volt; > + int sram_min_volt; > + int sram_max_volt; > +}; > + > +struct mtk_ccifreq_drv { > + struct device *dev; > + struct devfreq *devfreq; > + struct regulator *proc_reg; > + struct regulator *sram_reg; > + struct clk *cci_clk; > + struct clk *inter_clk; > + int inter_voltage; > + int pre_voltage; > + unsigned long pre_freq; > + /* Avoid race condition for regulators between notify and > policy */ > + struct mutex reg_lock; > + struct notifier_block opp_nb; > + const struct mtk_ccifreq_platform_data *soc_data; > + int vtrack_max; > +}; > + > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int > new_voltage) > +{ > + const struct mtk_ccifreq_platform_data *soc_data = drv- > >soc_data; > + struct device *dev = drv->dev; > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; > + int retry_max = drv->vtrack_max; > + > + if (!drv->sram_reg) { > + ret = regulator_set_voltage(drv->proc_reg, new_voltage, > + drv->soc_data- > >proc_max_volt); > + goto out_set_voltage; > + } > + > + pre_voltage = regulator_get_voltage(drv->proc_reg); > + if (pre_voltage < 0) { > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > + return pre_voltage; > + } > + > + pre_vsram = regulator_get_voltage(drv->sram_reg); > + if (pre_vsram < 0) { > + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); > + return pre_vsram; > + } > + > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > + soc_data->sram_min_volt, soc_data- > >sram_max_volt); > + > + do { > + if (pre_voltage <= new_voltage) { > + vsram = clamp(pre_voltage + soc_data- > >max_volt_shift, > + soc_data->sram_min_volt, > new_vsram); > + ret = regulator_set_voltage(drv->sram_reg, > vsram, > + soc_data- > >sram_max_volt); > + if (ret) > + return ret; > + > + if (vsram == soc_data->sram_max_volt || > + new_vsram == soc_data->sram_min_volt) > + voltage = new_voltage; > + else > + voltage = vsram - soc_data- > >min_volt_shift; > + > + ret = regulator_set_voltage(drv->proc_reg, > voltage, > + soc_data- > >proc_max_volt); > + if (ret) { > + regulator_set_voltage(drv->sram_reg, > pre_vsram, > + soc_data- > >sram_max_volt); > + return ret; > + } > + } else if (pre_voltage > new_voltage) { > + voltage = max(new_voltage, > + pre_vsram - soc_data- > >max_volt_shift); > + ret = regulator_set_voltage(drv->proc_reg, > voltage, > + soc_data- > >proc_max_volt); > + if (ret) > + return ret; > + > + if (voltage == new_voltage) > + vsram = new_vsram; > + else > + vsram = max(new_vsram, > + voltage + soc_data- > >min_volt_shift); > + > + ret = regulator_set_voltage(drv->sram_reg, > vsram, > + soc_data- > >sram_max_volt); > + if (ret) { > + regulator_set_voltage(drv->proc_reg, > pre_voltage, > + soc_data- > >proc_max_volt); > + return ret; > + } > + } > + > + pre_voltage = voltage; > + pre_vsram = vsram; > + > + if (--retry_max < 0) { > + dev_err(dev, > + "over loop count, failed to set > voltage\n"); > + return -EINVAL; > + } > + } while (voltage != new_voltage || vsram != new_vsram); > + > +out_set_voltage: > + if (!ret) > + drv->pre_voltage = new_voltage; > + > + return ret; > +} > + > +static int mtk_ccifreq_target(struct device *dev, unsigned long > *freq, > + u32 flags) > +{ > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > + struct dev_pm_opp *opp; > + unsigned long opp_rate; > + int voltage, pre_voltage, inter_voltage, target_voltage, ret; > + > + if (!drv) > + return -EINVAL; > + > + if (drv->pre_freq == *freq) > + return 0; > + > + inter_voltage = drv->inter_voltage; > + > + opp_rate = *freq; > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > + if (IS_ERR(opp)) { > + dev_err(dev, "failed to find opp for freq: %ld\n", > opp_rate); > + return PTR_ERR(opp); > + } > + > + mutex_lock(&drv->reg_lock); > + > + voltage = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + > + if (unlikely(drv->pre_voltage <= 0)) > + pre_voltage = regulator_get_voltage(drv->proc_reg); > + else > + pre_voltage = drv->pre_voltage; > + > + if (pre_voltage < 0) { > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > + return pre_voltage; > + } > + > + /* scale up: set voltage first then freq. */ > + target_voltage = max(inter_voltage, voltage); > + if (pre_voltage <= target_voltage) { > + ret = mtk_ccifreq_set_voltage(drv, target_voltage); > + if (ret) { > + dev_err(dev, "failed to scale up voltage\n"); > + goto out_restore_voltage; > + } > + } > + > + /* switch the cci clock to intermediate clock source. */ > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > + if (ret) { > + dev_err(dev, "failed to re-parent cci clock\n"); > + goto out_restore_voltage; > + } > + > + /* set the original clock to target rate. */ > + ret = clk_set_rate(cci_pll, *freq); > + if (ret) { > + dev_err(dev, "failed to set cci pll rate: %d\n", ret); > + clk_set_parent(drv->cci_clk, cci_pll); > + goto out_restore_voltage; > + } > + > + /* switch the cci clock back to the original clock source. */ > + ret = clk_set_parent(drv->cci_clk, cci_pll); > + if (ret) { > + dev_err(dev, "failed to re-parent cci clock\n"); > + mtk_ccifreq_set_voltage(drv, inter_voltage); > + goto out_unlock; > + } > + > + /* > + * If the new voltage is lower than the intermediate voltage or > the > + * original voltage, scale down to the new voltage. > + */ > + if (voltage < inter_voltage || voltage < pre_voltage) { > + ret = mtk_ccifreq_set_voltage(drv, voltage); > + if (ret) { > + dev_err(dev, "failed to scale down voltage\n"); > + goto out_unlock; > + } > + } > + > + drv->pre_freq = *freq; > + mutex_unlock(&drv->reg_lock); > + > + return 0; > + > +out_restore_voltage: > + mtk_ccifreq_set_voltage(drv, pre_voltage); > + > +out_unlock: > + mutex_unlock(&drv->reg_lock); > + return ret; > +} > + > +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct dev_pm_opp *opp = data; > + struct mtk_ccifreq_drv *drv; > + unsigned long freq, volt; > + > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > + > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > + freq = dev_pm_opp_get_freq(opp); > + > + mutex_lock(&drv->reg_lock); > + /* current opp item is changed */ > + if (freq == drv->pre_freq) { > + volt = dev_pm_opp_get_voltage(opp); > + mtk_ccifreq_set_voltage(drv, volt); > + } > + mutex_unlock(&drv->reg_lock); > + } > + > + return 0; > +} > + > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > + .target = mtk_ccifreq_target, > +}; > + > +static int mtk_ccifreq_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_ccifreq_drv *drv; > + struct devfreq_passive_data *passive_data; > + struct dev_pm_opp *opp; > + unsigned long rate, opp_volt; > + int ret; > + > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > + if (!drv) > + return -ENOMEM; > + > + drv->dev = dev; > + drv->soc_data = (const struct mtk_ccifreq_platform_data *) > + of_device_get_match_data(&pdev->dev); > + mutex_init(&drv->reg_lock); > + platform_set_drvdata(pdev, drv); > + > + drv->cci_clk = devm_clk_get(dev, "cci"); > + if (IS_ERR(drv->cci_clk)) { > + ret = PTR_ERR(drv->cci_clk); > + return dev_err_probe(dev, ret, > + "failed to get cci clk: %d\n", > ret); > + } > + > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > + if (IS_ERR(drv->inter_clk)) { > + ret = PTR_ERR(drv->inter_clk); > + dev_err_probe(dev, ret, > + "failed to get intermediate clk: %d\n", > ret); > + goto out_free_resources; > + } > + > + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); > + if (IS_ERR(drv->proc_reg)) { > + ret = PTR_ERR(drv->proc_reg); > + dev_err_probe(dev, ret, > + "failed to get proc regulator: %d\n", > ret); > + goto out_free_resources; > + } > + > + ret = regulator_enable(drv->proc_reg); > + if (ret) { > + dev_err(dev, "failed to enable proc regulator\n"); > + goto out_free_resources; > + } > + > + drv->sram_reg = regulator_get_optional(dev, "sram"); > + if (IS_ERR(drv->sram_reg)) > + drv->sram_reg = NULL; > + else { > + ret = regulator_enable(drv->sram_reg); > + if (ret) { > + dev_err(dev, "failed to enable sram > regulator\n"); > + goto out_free_resources; > + } > + } > + > + /* > + * We assume min voltage is 0 and tracking target voltage using > + * min_volt_shift for each iteration. > + * The retry_max is 3 times of expeted iteration count. > + */ > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > >sram_max_volt, > + drv->soc_data- > >proc_max_volt), > + drv->soc_data- > >min_volt_shift); > + > + ret = clk_prepare_enable(drv->cci_clk); > + if (ret) > + goto out_free_resources; > + > + ret = clk_prepare_enable(drv->inter_clk); > + if (ret) > + goto out_disable_cci_clk; > + > + ret = dev_pm_opp_of_add_table(dev); > + if (ret) { > + dev_err(dev, "failed to add opp table: %d\n", ret); > + goto out_disable_inter_clk; > + } > + > + rate = clk_get_rate(drv->inter_clk); > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > + if (IS_ERR(opp)) { > + ret = PTR_ERR(opp); > + dev_err(dev, "failed to get intermediate opp: %d\n", > ret); > + goto out_remove_opp_table; > + } > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + > + rate = U32_MAX; > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > + if (IS_ERR(opp)) { > + dev_err(dev, "failed to get opp\n"); > + ret = PTR_ERR(opp); > + goto out_remove_opp_table; > + } > + > + opp_volt = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > + if (ret) { > + dev_err(dev, "failed to scale to highest voltage %lu in > proc_reg\n", > + opp_volt); > + goto out_remove_opp_table; > + } > + > + passive_data = devm_kzalloc(dev, sizeof(struct > devfreq_passive_data), > + GFP_KERNEL); > + if (!passive_data) { > + ret = -ENOMEM; > + goto out_remove_opp_table; > + } > + > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > + drv->devfreq = devm_devfreq_add_device(dev, > &mtk_ccifreq_profile, > + DEVFREQ_GOV_PASSIVE, > + passive_data); > + if (IS_ERR(drv->devfreq)) { > + ret = -EPROBE_DEFER; > + dev_err(dev, "failed to add devfreq device: %d\n", > + PTR_ERR(drv->devfreq)); > + goto out_remove_opp_table; > + } > + > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > + if (ret) { > + dev_err(dev, "failed to register opp notifier: %d\n", > ret); > + goto out_remove_devfreq_device; > + } > + return 0; > + > +out_remove_devfreq_device: > + devm_devfreq_remove_device(dev, drv->devfreq); > + > +out_remove_opp_table: > + dev_pm_opp_of_remove_table(dev); > + > +out_disable_inter_clk: > + clk_disable_unprepare(drv->inter_clk); > + > +out_disable_cci_clk: > + clk_disable_unprepare(drv->cci_clk); > + > +out_free_resources: > + if (regulator_is_enabled(drv->proc_reg)) > + regulator_disable(drv->proc_reg); > + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) > + regulator_disable(drv->sram_reg); > + > + if (!IS_ERR(drv->proc_reg)) > + regulator_put(drv->proc_reg); > + if (!IS_ERR(drv->sram_reg)) > + regulator_put(drv->sram_reg); > + if (!IS_ERR(drv->cci_clk)) > + clk_put(drv->cci_clk); > + if (!IS_ERR(drv->inter_clk)) > + clk_put(drv->inter_clk); > + > + return ret; > +} > + > +static int mtk_ccifreq_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_ccifreq_drv *drv; > + > + drv = platform_get_drvdata(pdev); > + > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > + dev_pm_opp_of_remove_table(dev); > + clk_disable_unprepare(drv->inter_clk); > + clk_disable_unprepare(drv->cci_clk); > + regulator_disable(drv->proc_reg); > + if (drv->sram_reg) > + regulator_disable(drv->sram_reg); > + > + return 0; > +} > + > +static const struct mtk_ccifreq_platform_data mt8183_platform_data = > { > + .min_volt_shift = 100000, > + .max_volt_shift = 200000, > + .proc_max_volt = 1150000, > + .sram_min_volt = 0, > + .sram_max_volt = 1150000, > +}; > + > +static const struct mtk_ccifreq_platform_data mt8186_platform_data = > { > + .min_volt_shift = 100000, > + .max_volt_shift = 250000, > + .proc_max_volt = 1118750, > + .sram_min_volt = 850000, > + .sram_max_volt = 1118750, > +}; > + > +static const struct of_device_id mtk_ccifreq_machines[] = { > + { .compatible = "mediatek,mt8183-cci", .data = > &mt8183_platform_data }, > + { .compatible = "mediatek,mt8186-cci", .data = > &mt8186_platform_data }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > + > +static struct platform_driver mtk_ccifreq_platdrv = { > + .probe = mtk_ccifreq_probe, > + .remove = mtk_ccifreq_remove, > + .driver = { > + .name = "mtk-ccifreq", > + .of_match_table = mtk_ccifreq_machines, > + }, > +}; > +module_platform_driver(mtk_ccifreq_platdrv); > + > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > +MODULE_LICENSE("GPL v2"); Hi Chanwoo, Just a kindly ping. Could you please give me some suggestion on this patch? Thanks! BRs, Johnson Wang
On 22. 5. 6. 20:38, Johnson Wang wrote: > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: >> We introduce a devfreq driver for the MediaTek Cache Coherent >> Interconnect >> (CCI) used by some MediaTek SoCs. >> >> In this driver, we use the passive devfreq driver to get target >> frequencies >> and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek >> CCI >> is supplied by the same regulators with the little core CPUs. >> >> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> >> --- >> This patch depends on "devfreq-testing"[1]. >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing >> --- >> drivers/devfreq/Kconfig | 10 + >> drivers/devfreq/Makefile | 1 + >> drivers/devfreq/mtk-cci-devfreq.c | 474 >> ++++++++++++++++++++++++++++++ >> 3 files changed, 485 insertions(+) >> create mode 100644 drivers/devfreq/mtk-cci-devfreq.c >> >> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >> index 87eb2b837e68..9754d8b31621 100644 >> --- a/drivers/devfreq/Kconfig >> +++ b/drivers/devfreq/Kconfig >> @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ >> It reads ACTMON counters of memory controllers and adjusts >> the >> operating frequencies and voltages with OPP support. >> >> +config ARM_MEDIATEK_CCI_DEVFREQ >> + tristate "MEDIATEK CCI DEVFREQ Driver" >> + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST >> + select DEVFREQ_GOV_PASSIVE >> + help >> + This adds a devfreq driver for MediaTek Cache Coherent >> Interconnect >> + which is shared the same regulators with the cpu cluster. It >> can track >> + buck voltages and update a proper CCI frequency. Use the >> notification >> + to get the regulator status. >> + >> config ARM_RK3399_DMC_DEVFREQ >> tristate "ARM RK3399 DMC DEVFREQ Driver" >> depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ >> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile >> index 0b6be92a25d9..bf40d04928d0 100644 >> --- a/drivers/devfreq/Makefile >> +++ b/drivers/devfreq/Makefile >> @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += >> governor_passive.o >> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o >> obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o >> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o >> +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o >> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o >> obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o >> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o >> diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk- >> cci-devfreq.c >> new file mode 100644 >> index 000000000000..b3e31c45a57c >> --- /dev/null >> +++ b/drivers/devfreq/mtk-cci-devfreq.c >> @@ -0,0 +1,474 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (C) 2022 MediaTek Inc. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/devfreq.h> >> +#include <linux/minmax.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_opp.h> >> +#include <linux/regulator/consumer.h> >> + >> +struct mtk_ccifreq_platform_data { >> + int min_volt_shift; >> + int max_volt_shift; >> + int proc_max_volt; >> + int sram_min_volt; >> + int sram_max_volt; >> +}; >> + >> +struct mtk_ccifreq_drv { >> + struct device *dev; >> + struct devfreq *devfreq; >> + struct regulator *proc_reg; >> + struct regulator *sram_reg; >> + struct clk *cci_clk; >> + struct clk *inter_clk; >> + int inter_voltage; >> + int pre_voltage; >> + unsigned long pre_freq; >> + /* Avoid race condition for regulators between notify and >> policy */ >> + struct mutex reg_lock; >> + struct notifier_block opp_nb; >> + const struct mtk_ccifreq_platform_data *soc_data; >> + int vtrack_max; >> +}; >> + >> +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int >> new_voltage) >> +{ >> + const struct mtk_ccifreq_platform_data *soc_data = drv- >>> soc_data; >> + struct device *dev = drv->dev; >> + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; >> + int retry_max = drv->vtrack_max; >> + >> + if (!drv->sram_reg) { >> + ret = regulator_set_voltage(drv->proc_reg, new_voltage, >> + drv->soc_data- >>> proc_max_volt); >> + goto out_set_voltage; >> + } >> + >> + pre_voltage = regulator_get_voltage(drv->proc_reg); >> + if (pre_voltage < 0) { >> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >> + return pre_voltage; >> + } >> + >> + pre_vsram = regulator_get_voltage(drv->sram_reg); >> + if (pre_vsram < 0) { >> + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); >> + return pre_vsram; >> + } >> + >> + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, >> + soc_data->sram_min_volt, soc_data- >>> sram_max_volt); >> + >> + do { >> + if (pre_voltage <= new_voltage) { >> + vsram = clamp(pre_voltage + soc_data- >>> max_volt_shift, >> + soc_data->sram_min_volt, >> new_vsram); >> + ret = regulator_set_voltage(drv->sram_reg, >> vsram, >> + soc_data- >>> sram_max_volt); >> + if (ret) >> + return ret; >> + >> + if (vsram == soc_data->sram_max_volt || >> + new_vsram == soc_data->sram_min_volt) >> + voltage = new_voltage; >> + else >> + voltage = vsram - soc_data- >>> min_volt_shift; >> + >> + ret = regulator_set_voltage(drv->proc_reg, >> voltage, >> + soc_data- >>> proc_max_volt); >> + if (ret) { >> + regulator_set_voltage(drv->sram_reg, >> pre_vsram, >> + soc_data- >>> sram_max_volt); >> + return ret; >> + } >> + } else if (pre_voltage > new_voltage) { >> + voltage = max(new_voltage, >> + pre_vsram - soc_data- >>> max_volt_shift); >> + ret = regulator_set_voltage(drv->proc_reg, >> voltage, >> + soc_data- >>> proc_max_volt); >> + if (ret) >> + return ret; >> + >> + if (voltage == new_voltage) >> + vsram = new_vsram; >> + else >> + vsram = max(new_vsram, >> + voltage + soc_data- >>> min_volt_shift); >> + >> + ret = regulator_set_voltage(drv->sram_reg, >> vsram, >> + soc_data- >>> sram_max_volt); >> + if (ret) { >> + regulator_set_voltage(drv->proc_reg, >> pre_voltage, >> + soc_data- >>> proc_max_volt); >> + return ret; >> + } >> + } >> + >> + pre_voltage = voltage; >> + pre_vsram = vsram; >> + >> + if (--retry_max < 0) { >> + dev_err(dev, >> + "over loop count, failed to set >> voltage\n"); >> + return -EINVAL; >> + } >> + } while (voltage != new_voltage || vsram != new_vsram); >> + >> +out_set_voltage: >> + if (!ret) >> + drv->pre_voltage = new_voltage; >> + >> + return ret; >> +} >> + >> +static int mtk_ccifreq_target(struct device *dev, unsigned long >> *freq, >> + u32 flags) >> +{ >> + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); >> + struct clk *cci_pll = clk_get_parent(drv->cci_clk); >> + struct dev_pm_opp *opp; >> + unsigned long opp_rate; >> + int voltage, pre_voltage, inter_voltage, target_voltage, ret; >> + >> + if (!drv) >> + return -EINVAL; >> + >> + if (drv->pre_freq == *freq) >> + return 0; >> + >> + inter_voltage = drv->inter_voltage; >> + >> + opp_rate = *freq; >> + opp = devfreq_recommended_opp(dev, &opp_rate, 1); >> + if (IS_ERR(opp)) { >> + dev_err(dev, "failed to find opp for freq: %ld\n", >> opp_rate); >> + return PTR_ERR(opp); >> + } >> + >> + mutex_lock(&drv->reg_lock); >> + >> + voltage = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + >> + if (unlikely(drv->pre_voltage <= 0)) >> + pre_voltage = regulator_get_voltage(drv->proc_reg); >> + else >> + pre_voltage = drv->pre_voltage; >> + >> + if (pre_voltage < 0) { >> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >> + return pre_voltage; >> + } >> + >> + /* scale up: set voltage first then freq. */ >> + target_voltage = max(inter_voltage, voltage); >> + if (pre_voltage <= target_voltage) { >> + ret = mtk_ccifreq_set_voltage(drv, target_voltage); >> + if (ret) { >> + dev_err(dev, "failed to scale up voltage\n"); >> + goto out_restore_voltage; >> + } >> + } >> + >> + /* switch the cci clock to intermediate clock source. */ >> + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); >> + if (ret) { >> + dev_err(dev, "failed to re-parent cci clock\n"); >> + goto out_restore_voltage; >> + } >> + >> + /* set the original clock to target rate. */ >> + ret = clk_set_rate(cci_pll, *freq); >> + if (ret) { >> + dev_err(dev, "failed to set cci pll rate: %d\n", ret); >> + clk_set_parent(drv->cci_clk, cci_pll); >> + goto out_restore_voltage; >> + } >> + >> + /* switch the cci clock back to the original clock source. */ >> + ret = clk_set_parent(drv->cci_clk, cci_pll); >> + if (ret) { >> + dev_err(dev, "failed to re-parent cci clock\n"); >> + mtk_ccifreq_set_voltage(drv, inter_voltage); >> + goto out_unlock; >> + } >> + >> + /* >> + * If the new voltage is lower than the intermediate voltage or >> the >> + * original voltage, scale down to the new voltage. >> + */ >> + if (voltage < inter_voltage || voltage < pre_voltage) { >> + ret = mtk_ccifreq_set_voltage(drv, voltage); >> + if (ret) { >> + dev_err(dev, "failed to scale down voltage\n"); >> + goto out_unlock; >> + } >> + } >> + >> + drv->pre_freq = *freq; >> + mutex_unlock(&drv->reg_lock); >> + >> + return 0; >> + >> +out_restore_voltage: >> + mtk_ccifreq_set_voltage(drv, pre_voltage); >> + >> +out_unlock: >> + mutex_unlock(&drv->reg_lock); >> + return ret; >> +} >> + >> +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, >> + unsigned long event, void *data) >> +{ >> + struct dev_pm_opp *opp = data; >> + struct mtk_ccifreq_drv *drv; >> + unsigned long freq, volt; >> + >> + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); >> + >> + if (event == OPP_EVENT_ADJUST_VOLTAGE) { >> + freq = dev_pm_opp_get_freq(opp); >> + >> + mutex_lock(&drv->reg_lock); >> + /* current opp item is changed */ >> + if (freq == drv->pre_freq) { >> + volt = dev_pm_opp_get_voltage(opp); >> + mtk_ccifreq_set_voltage(drv, volt); >> + } >> + mutex_unlock(&drv->reg_lock); >> + } >> + >> + return 0; >> +} >> + >> +static struct devfreq_dev_profile mtk_ccifreq_profile = { >> + .target = mtk_ccifreq_target, >> +}; >> + >> +static int mtk_ccifreq_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct mtk_ccifreq_drv *drv; >> + struct devfreq_passive_data *passive_data; >> + struct dev_pm_opp *opp; >> + unsigned long rate, opp_volt; >> + int ret; >> + >> + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); >> + if (!drv) >> + return -ENOMEM; >> + >> + drv->dev = dev; >> + drv->soc_data = (const struct mtk_ccifreq_platform_data *) >> + of_device_get_match_data(&pdev->dev); >> + mutex_init(&drv->reg_lock); >> + platform_set_drvdata(pdev, drv); >> + >> + drv->cci_clk = devm_clk_get(dev, "cci"); >> + if (IS_ERR(drv->cci_clk)) { >> + ret = PTR_ERR(drv->cci_clk); >> + return dev_err_probe(dev, ret, >> + "failed to get cci clk: %d\n", >> ret); >> + } >> + >> + drv->inter_clk = devm_clk_get(dev, "intermediate"); >> + if (IS_ERR(drv->inter_clk)) { >> + ret = PTR_ERR(drv->inter_clk); >> + dev_err_probe(dev, ret, >> + "failed to get intermediate clk: %d\n", >> ret); >> + goto out_free_resources; >> + } >> + >> + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); >> + if (IS_ERR(drv->proc_reg)) { >> + ret = PTR_ERR(drv->proc_reg); >> + dev_err_probe(dev, ret, >> + "failed to get proc regulator: %d\n", >> ret); >> + goto out_free_resources; >> + } >> + >> + ret = regulator_enable(drv->proc_reg); >> + if (ret) { >> + dev_err(dev, "failed to enable proc regulator\n"); >> + goto out_free_resources; >> + } >> + >> + drv->sram_reg = regulator_get_optional(dev, "sram"); >> + if (IS_ERR(drv->sram_reg)) >> + drv->sram_reg = NULL; >> + else { >> + ret = regulator_enable(drv->sram_reg); >> + if (ret) { >> + dev_err(dev, "failed to enable sram >> regulator\n"); >> + goto out_free_resources; >> + } >> + } >> + >> + /* >> + * We assume min voltage is 0 and tracking target voltage using >> + * min_volt_shift for each iteration. >> + * The retry_max is 3 times of expeted iteration count. >> + */ >> + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- >>> sram_max_volt, >> + drv->soc_data- >>> proc_max_volt), >> + drv->soc_data- >>> min_volt_shift); >> + >> + ret = clk_prepare_enable(drv->cci_clk); >> + if (ret) >> + goto out_free_resources; >> + >> + ret = clk_prepare_enable(drv->inter_clk); >> + if (ret) >> + goto out_disable_cci_clk; >> + >> + ret = dev_pm_opp_of_add_table(dev); >> + if (ret) { >> + dev_err(dev, "failed to add opp table: %d\n", ret); >> + goto out_disable_inter_clk; >> + } >> + >> + rate = clk_get_rate(drv->inter_clk); >> + opp = dev_pm_opp_find_freq_ceil(dev, &rate); >> + if (IS_ERR(opp)) { >> + ret = PTR_ERR(opp); >> + dev_err(dev, "failed to get intermediate opp: %d\n", >> ret); >> + goto out_remove_opp_table; >> + } >> + drv->inter_voltage = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + >> + rate = U32_MAX; >> + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); >> + if (IS_ERR(opp)) { >> + dev_err(dev, "failed to get opp\n"); >> + ret = PTR_ERR(opp); >> + goto out_remove_opp_table; >> + } >> + >> + opp_volt = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + ret = mtk_ccifreq_set_voltage(drv, opp_volt); >> + if (ret) { >> + dev_err(dev, "failed to scale to highest voltage %lu in >> proc_reg\n", >> + opp_volt); >> + goto out_remove_opp_table; >> + } >> + >> + passive_data = devm_kzalloc(dev, sizeof(struct >> devfreq_passive_data), >> + GFP_KERNEL); >> + if (!passive_data) { >> + ret = -ENOMEM; >> + goto out_remove_opp_table; >> + } >> + >> + passive_data->parent_type = CPUFREQ_PARENT_DEV; >> + drv->devfreq = devm_devfreq_add_device(dev, >> &mtk_ccifreq_profile, >> + DEVFREQ_GOV_PASSIVE, >> + passive_data); >> + if (IS_ERR(drv->devfreq)) { >> + ret = -EPROBE_DEFER; >> + dev_err(dev, "failed to add devfreq device: %d\n", >> + PTR_ERR(drv->devfreq)); >> + goto out_remove_opp_table; >> + } >> + >> + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; >> + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); >> + if (ret) { >> + dev_err(dev, "failed to register opp notifier: %d\n", >> ret); >> + goto out_remove_devfreq_device; >> + } >> + return 0; >> + >> +out_remove_devfreq_device: >> + devm_devfreq_remove_device(dev, drv->devfreq); >> + >> +out_remove_opp_table: >> + dev_pm_opp_of_remove_table(dev); >> + >> +out_disable_inter_clk: >> + clk_disable_unprepare(drv->inter_clk); >> + >> +out_disable_cci_clk: >> + clk_disable_unprepare(drv->cci_clk); >> + >> +out_free_resources: >> + if (regulator_is_enabled(drv->proc_reg)) >> + regulator_disable(drv->proc_reg); >> + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) >> + regulator_disable(drv->sram_reg); >> + >> + if (!IS_ERR(drv->proc_reg)) >> + regulator_put(drv->proc_reg); >> + if (!IS_ERR(drv->sram_reg)) >> + regulator_put(drv->sram_reg); >> + if (!IS_ERR(drv->cci_clk)) >> + clk_put(drv->cci_clk); >> + if (!IS_ERR(drv->inter_clk)) >> + clk_put(drv->inter_clk); >> + >> + return ret; >> +} >> + >> +static int mtk_ccifreq_remove(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct mtk_ccifreq_drv *drv; >> + >> + drv = platform_get_drvdata(pdev); >> + >> + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); >> + dev_pm_opp_of_remove_table(dev); >> + clk_disable_unprepare(drv->inter_clk); >> + clk_disable_unprepare(drv->cci_clk); >> + regulator_disable(drv->proc_reg); >> + if (drv->sram_reg) >> + regulator_disable(drv->sram_reg); >> + >> + return 0; >> +} >> + >> +static const struct mtk_ccifreq_platform_data mt8183_platform_data = >> { >> + .min_volt_shift = 100000, >> + .max_volt_shift = 200000, >> + .proc_max_volt = 1150000, >> + .sram_min_volt = 0, >> + .sram_max_volt = 1150000, >> +}; >> + >> +static const struct mtk_ccifreq_platform_data mt8186_platform_data = >> { >> + .min_volt_shift = 100000, >> + .max_volt_shift = 250000, >> + .proc_max_volt = 1118750, >> + .sram_min_volt = 850000, >> + .sram_max_volt = 1118750, >> +}; >> + >> +static const struct of_device_id mtk_ccifreq_machines[] = { >> + { .compatible = "mediatek,mt8183-cci", .data = >> &mt8183_platform_data }, >> + { .compatible = "mediatek,mt8186-cci", .data = >> &mt8186_platform_data }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); >> + >> +static struct platform_driver mtk_ccifreq_platdrv = { >> + .probe = mtk_ccifreq_probe, >> + .remove = mtk_ccifreq_remove, >> + .driver = { >> + .name = "mtk-ccifreq", >> + .of_match_table = mtk_ccifreq_machines, >> + }, >> +}; >> +module_platform_driver(mtk_ccifreq_platdrv); >> + >> +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); >> +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); >> +MODULE_LICENSE("GPL v2"); > > Hi Chanwoo, > > Just a kindly ping. > Could you please give me some suggestion on this patch? > Thanks! > > BRs, > Johnson Wang > Hi Johnson, Sorry for late reply.But I replied it yesterday as following: Maybe, this reply[1] has not yet arrrived at your mail box. [1] https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef As I described on reply[1], I updated the patches on devfreq-testing branch. Could you please test your patches based on devfreq-testing branch? -- Best Regards, Samsung Electronics Chanwoo Choi
On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > On 22. 5. 6. 20:38, Johnson Wang wrote: > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > Interconnect > > > (CCI) used by some MediaTek SoCs. > > > > > > In this driver, we use the passive devfreq driver to get target > > > frequencies > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > MediaTek > > > CCI > > > is supplied by the same regulators with the little core CPUs. > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > --- > > > This patch depends on "devfreq-testing"[1]. > > > [1] > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > --- > > > drivers/devfreq/Kconfig | 10 + > > > drivers/devfreq/Makefile | 1 + > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > ++++++++++++++++++++++++++++++ > > > 3 files changed, 485 insertions(+) > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > > > index 87eb2b837e68..9754d8b31621 100644 > > > --- a/drivers/devfreq/Kconfig > > > +++ b/drivers/devfreq/Kconfig > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > It reads ACTMON counters of memory controllers and > > > adjusts > > > the > > > operating frequencies and voltages with OPP support. > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > + select DEVFREQ_GOV_PASSIVE > > > + help > > > + This adds a devfreq driver for MediaTek Cache Coherent > > > Interconnect > > > + which is shared the same regulators with the cpu cluster. It > > > can track > > > + buck voltages and update a proper CCI frequency. Use the > > > notification > > > + to get the regulator status. > > > + > > > config ARM_RK3399_DMC_DEVFREQ > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > --- a/drivers/devfreq/Makefile > > > +++ b/drivers/devfreq/Makefile > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > governor_passive.o > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > mbus.o > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > b/drivers/devfreq/mtk- > > > cci-devfreq.c > > > new file mode 100644 > > > index 000000000000..b3e31c45a57c > > > --- /dev/null > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > @@ -0,0 +1,474 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (C) 2022 MediaTek Inc. > > > + */ > > > + > > > +#include <linux/clk.h> > > > +#include <linux/devfreq.h> > > > +#include <linux/minmax.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/pm_opp.h> > > > +#include <linux/regulator/consumer.h> > > > + > > > +struct mtk_ccifreq_platform_data { > > > + int min_volt_shift; > > > + int max_volt_shift; > > > + int proc_max_volt; > > > + int sram_min_volt; > > > + int sram_max_volt; > > > +}; > > > + > > > +struct mtk_ccifreq_drv { > > > + struct device *dev; > > > + struct devfreq *devfreq; > > > + struct regulator *proc_reg; > > > + struct regulator *sram_reg; > > > + struct clk *cci_clk; > > > + struct clk *inter_clk; > > > + int inter_voltage; > > > + int pre_voltage; > > > + unsigned long pre_freq; > > > + /* Avoid race condition for regulators between notify and > > > policy */ > > > + struct mutex reg_lock; > > > + struct notifier_block opp_nb; > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > + int vtrack_max; > > > +}; > > > + > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, > > > int > > > new_voltage) > > > +{ > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > soc_data; > > > > > > + struct device *dev = drv->dev; > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; > > > + int retry_max = drv->vtrack_max; > > > + > > > + if (!drv->sram_reg) { > > > + ret = regulator_set_voltage(drv->proc_reg, new_voltage, > > > + drv->soc_data- > > > > proc_max_volt); > > > > > > + goto out_set_voltage; > > > + } > > > + > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > + if (pre_voltage < 0) { > > > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > > > + return pre_voltage; > > > + } > > > + > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > + if (pre_vsram < 0) { > > > + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); > > > + return pre_vsram; > > > + } > > > + > > > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > > > + soc_data->sram_min_volt, soc_data- > > > > sram_max_volt); > > > > > > + > > > + do { > > > + if (pre_voltage <= new_voltage) { > > > + vsram = clamp(pre_voltage + soc_data- > > > > max_volt_shift, > > > > > > + soc_data->sram_min_volt, > > > new_vsram); > > > + ret = regulator_set_voltage(drv->sram_reg, > > > vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + if (ret) > > > + return ret; > > > + > > > + if (vsram == soc_data->sram_max_volt || > > > + new_vsram == soc_data->sram_min_volt) > > > + voltage = new_voltage; > > > + else > > > + voltage = vsram - soc_data- > > > > min_volt_shift; > > > > > > + > > > + ret = regulator_set_voltage(drv->proc_reg, > > > voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + if (ret) { > > > + regulator_set_voltage(drv->sram_reg, > > > pre_vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + return ret; > > > + } > > > + } else if (pre_voltage > new_voltage) { > > > + voltage = max(new_voltage, > > > + pre_vsram - soc_data- > > > > max_volt_shift); > > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + if (ret) > > > + return ret; > > > + > > > + if (voltage == new_voltage) > > > + vsram = new_vsram; > > > + else > > > + vsram = max(new_vsram, > > > + voltage + soc_data- > > > > min_volt_shift); > > > > > > + > > > + ret = regulator_set_voltage(drv->sram_reg, > > > vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + if (ret) { > > > + regulator_set_voltage(drv->proc_reg, > > > pre_voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + return ret; > > > + } > > > + } > > > + > > > + pre_voltage = voltage; > > > + pre_vsram = vsram; > > > + > > > + if (--retry_max < 0) { > > > + dev_err(dev, > > > + "over loop count, failed to set > > > voltage\n"); > > > + return -EINVAL; > > > + } > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > + > > > +out_set_voltage: > > > + if (!ret) > > > + drv->pre_voltage = new_voltage; > > > + > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_target(struct device *dev, unsigned long > > > *freq, > > > + u32 flags) > > > +{ > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > + struct dev_pm_opp *opp; > > > + unsigned long opp_rate; > > > + int voltage, pre_voltage, inter_voltage, target_voltage, ret; > > > + > > > + if (!drv) > > > + return -EINVAL; > > > + > > > + if (drv->pre_freq == *freq) > > > + return 0; > > > + > > > + inter_voltage = drv->inter_voltage; > > > + > > > + opp_rate = *freq; > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > + if (IS_ERR(opp)) { > > > + dev_err(dev, "failed to find opp for freq: %ld\n", > > > opp_rate); > > > + return PTR_ERR(opp); > > > + } > > > + > > > + mutex_lock(&drv->reg_lock); > > > + > > > + voltage = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + > > > + if (unlikely(drv->pre_voltage <= 0)) > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > + else > > > + pre_voltage = drv->pre_voltage; > > > + > > > + if (pre_voltage < 0) { > > > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > > > + return pre_voltage; > > > + } > > > + > > > + /* scale up: set voltage first then freq. */ > > > + target_voltage = max(inter_voltage, voltage); > > > + if (pre_voltage <= target_voltage) { > > > + ret = mtk_ccifreq_set_voltage(drv, target_voltage); > > > + if (ret) { > > > + dev_err(dev, "failed to scale up voltage\n"); > > > + goto out_restore_voltage; > > > + } > > > + } > > > + > > > + /* switch the cci clock to intermediate clock source. */ > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > + if (ret) { > > > + dev_err(dev, "failed to re-parent cci clock\n"); > > > + goto out_restore_voltage; > > > + } > > > + > > > + /* set the original clock to target rate. */ > > > + ret = clk_set_rate(cci_pll, *freq); > > > + if (ret) { > > > + dev_err(dev, "failed to set cci pll rate: %d\n", ret); > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > + goto out_restore_voltage; > > > + } > > > + > > > + /* switch the cci clock back to the original clock source. */ > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > + if (ret) { > > > + dev_err(dev, "failed to re-parent cci clock\n"); > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > + goto out_unlock; > > > + } > > > + > > > + /* > > > + * If the new voltage is lower than the intermediate voltage or > > > the > > > + * original voltage, scale down to the new voltage. > > > + */ > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > + if (ret) { > > > + dev_err(dev, "failed to scale down voltage\n"); > > > + goto out_unlock; > > > + } > > > + } > > > + > > > + drv->pre_freq = *freq; > > > + mutex_unlock(&drv->reg_lock); > > > + > > > + return 0; > > > + > > > +out_restore_voltage: > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > + > > > +out_unlock: > > > + mutex_unlock(&drv->reg_lock); > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, > > > + unsigned long event, void *data) > > > +{ > > > + struct dev_pm_opp *opp = data; > > > + struct mtk_ccifreq_drv *drv; > > > + unsigned long freq, volt; > > > + > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > + > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > + freq = dev_pm_opp_get_freq(opp); > > > + > > > + mutex_lock(&drv->reg_lock); > > > + /* current opp item is changed */ > > > + if (freq == drv->pre_freq) { > > > + volt = dev_pm_opp_get_voltage(opp); > > > + mtk_ccifreq_set_voltage(drv, volt); > > > + } > > > + mutex_unlock(&drv->reg_lock); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > + .target = mtk_ccifreq_target, > > > +}; > > > + > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct mtk_ccifreq_drv *drv; > > > + struct devfreq_passive_data *passive_data; > > > + struct dev_pm_opp *opp; > > > + unsigned long rate, opp_volt; > > > + int ret; > > > + > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > + if (!drv) > > > + return -ENOMEM; > > > + > > > + drv->dev = dev; > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data *) > > > + of_device_get_match_data(&pdev->dev); > > > + mutex_init(&drv->reg_lock); > > > + platform_set_drvdata(pdev, drv); > > > + > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > + if (IS_ERR(drv->cci_clk)) { > > > + ret = PTR_ERR(drv->cci_clk); > > > + return dev_err_probe(dev, ret, > > > + "failed to get cci clk: %d\n", > > > ret); > > > + } > > > + > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > + if (IS_ERR(drv->inter_clk)) { > > > + ret = PTR_ERR(drv->inter_clk); > > > + dev_err_probe(dev, ret, > > > + "failed to get intermediate clk: %d\n", > > > ret); > > > + goto out_free_resources; > > > + } > > > + > > > + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); > > > + if (IS_ERR(drv->proc_reg)) { > > > + ret = PTR_ERR(drv->proc_reg); > > > + dev_err_probe(dev, ret, > > > + "failed to get proc regulator: %d\n", > > > ret); > > > + goto out_free_resources; > > > + } > > > + > > > + ret = regulator_enable(drv->proc_reg); > > > + if (ret) { > > > + dev_err(dev, "failed to enable proc regulator\n"); > > > + goto out_free_resources; > > > + } > > > + > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > + if (IS_ERR(drv->sram_reg)) > > > + drv->sram_reg = NULL; > > > + else { > > > + ret = regulator_enable(drv->sram_reg); > > > + if (ret) { > > > + dev_err(dev, "failed to enable sram > > > regulator\n"); > > > + goto out_free_resources; > > > + } > > > + } > > > + > > > + /* > > > + * We assume min voltage is 0 and tracking target voltage using > > > + * min_volt_shift for each iteration. > > > + * The retry_max is 3 times of expeted iteration count. > > > + */ > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > sram_max_volt, > > > > > > + drv->soc_data- > > > > proc_max_volt), > > > > > > + drv->soc_data- > > > > min_volt_shift); > > > > > > + > > > + ret = clk_prepare_enable(drv->cci_clk); > > > + if (ret) > > > + goto out_free_resources; > > > + > > > + ret = clk_prepare_enable(drv->inter_clk); > > > + if (ret) > > > + goto out_disable_cci_clk; > > > + > > > + ret = dev_pm_opp_of_add_table(dev); > > > + if (ret) { > > > + dev_err(dev, "failed to add opp table: %d\n", ret); > > > + goto out_disable_inter_clk; > > > + } > > > + > > > + rate = clk_get_rate(drv->inter_clk); > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > + if (IS_ERR(opp)) { > > > + ret = PTR_ERR(opp); > > > + dev_err(dev, "failed to get intermediate opp: %d\n", > > > ret); > > > + goto out_remove_opp_table; > > > + } > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + > > > + rate = U32_MAX; > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > + if (IS_ERR(opp)) { > > > + dev_err(dev, "failed to get opp\n"); > > > + ret = PTR_ERR(opp); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > + if (ret) { > > > + dev_err(dev, "failed to scale to highest voltage %lu in > > > proc_reg\n", > > > + opp_volt); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > devfreq_passive_data), > > > + GFP_KERNEL); > > > + if (!passive_data) { > > > + ret = -ENOMEM; > > > + goto out_remove_opp_table; > > > + } > > > + > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > &mtk_ccifreq_profile, > > > + DEVFREQ_GOV_PASSIVE, > > > + passive_data); > > > + if (IS_ERR(drv->devfreq)) { > > > + ret = -EPROBE_DEFER; > > > + dev_err(dev, "failed to add devfreq device: %d\n", > > > + PTR_ERR(drv->devfreq)); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > + if (ret) { > > > + dev_err(dev, "failed to register opp notifier: %d\n", > > > ret); > > > + goto out_remove_devfreq_device; > > > + } > > > + return 0; > > > + > > > +out_remove_devfreq_device: > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > + > > > +out_remove_opp_table: > > > + dev_pm_opp_of_remove_table(dev); > > > + > > > +out_disable_inter_clk: > > > + clk_disable_unprepare(drv->inter_clk); > > > + > > > +out_disable_cci_clk: > > > + clk_disable_unprepare(drv->cci_clk); > > > + > > > +out_free_resources: > > > + if (regulator_is_enabled(drv->proc_reg)) > > > + regulator_disable(drv->proc_reg); > > > + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) > > > + regulator_disable(drv->sram_reg); > > > + > > > + if (!IS_ERR(drv->proc_reg)) > > > + regulator_put(drv->proc_reg); > > > + if (!IS_ERR(drv->sram_reg)) > > > + regulator_put(drv->sram_reg); > > > + if (!IS_ERR(drv->cci_clk)) > > > + clk_put(drv->cci_clk); > > > + if (!IS_ERR(drv->inter_clk)) > > > + clk_put(drv->inter_clk); > > > + > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct mtk_ccifreq_drv *drv; > > > + > > > + drv = platform_get_drvdata(pdev); > > > + > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > + dev_pm_opp_of_remove_table(dev); > > > + clk_disable_unprepare(drv->inter_clk); > > > + clk_disable_unprepare(drv->cci_clk); > > > + regulator_disable(drv->proc_reg); > > > + if (drv->sram_reg) > > > + regulator_disable(drv->sram_reg); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct mtk_ccifreq_platform_data > > > mt8183_platform_data = > > > { > > > + .min_volt_shift = 100000, > > > + .max_volt_shift = 200000, > > > + .proc_max_volt = 1150000, > > > + .sram_min_volt = 0, > > > + .sram_max_volt = 1150000, > > > +}; > > > + > > > +static const struct mtk_ccifreq_platform_data > > > mt8186_platform_data = > > > { > > > + .min_volt_shift = 100000, > > > + .max_volt_shift = 250000, > > > + .proc_max_volt = 1118750, > > > + .sram_min_volt = 850000, > > > + .sram_max_volt = 1118750, > > > +}; > > > + > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > &mt8183_platform_data }, > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > &mt8186_platform_data }, > > > + { }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > + > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > + .probe = mtk_ccifreq_probe, > > > + .remove = mtk_ccifreq_remove, > > > + .driver = { > > > + .name = "mtk-ccifreq", > > > + .of_match_table = mtk_ccifreq_machines, > > > + }, > > > +}; > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > + > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > +MODULE_LICENSE("GPL v2"); > > > > Hi Chanwoo, > > > > Just a kindly ping. > > Could you please give me some suggestion on this patch? > > Thanks! > > > > BRs, > > Johnson Wang > > > > Hi Johnson, > > Sorry for late reply.But I replied it yesterday as following: > Maybe, this reply[1] has not yet arrrived at your mail box. > [1] > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > As I described on reply[1], I updated the patches on devfreq-testing > branch. Could you please test your patches based on devfreq-testing > branch? > Hi Chanwoo, Thanks for your information. I've tested this patch based on the latest devfreq-testing branch. It encounters the same crash as Chen-Yu mentioned[1]. [1]https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u BRs, Johnson Wang
On 22. 5. 9. 14:57, Johnson Wang wrote: > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: >> On 22. 5. 6. 20:38, Johnson Wang wrote: >>> On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: >>>> We introduce a devfreq driver for the MediaTek Cache Coherent >>>> Interconnect >>>> (CCI) used by some MediaTek SoCs. >>>> >>>> In this driver, we use the passive devfreq driver to get target >>>> frequencies >>>> and adjust voltages accordingly. In MT8183 and MT8186, the >>>> MediaTek >>>> CCI >>>> is supplied by the same regulators with the little core CPUs. >>>> >>>> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >>>> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> >>>> --- >>>> This patch depends on "devfreq-testing"[1]. >>>> [1] >>>> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ >>>> >>>> --- >>>> drivers/devfreq/Kconfig | 10 + >>>> drivers/devfreq/Makefile | 1 + >>>> drivers/devfreq/mtk-cci-devfreq.c | 474 >>>> ++++++++++++++++++++++++++++++ >>>> 3 files changed, 485 insertions(+) >>>> create mode 100644 drivers/devfreq/mtk-cci-devfreq.c >>>> >>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >>>> index 87eb2b837e68..9754d8b31621 100644 >>>> --- a/drivers/devfreq/Kconfig >>>> +++ b/drivers/devfreq/Kconfig >>>> @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ >>>> It reads ACTMON counters of memory controllers and >>>> adjusts >>>> the >>>> operating frequencies and voltages with OPP support. >>>> >>>> +config ARM_MEDIATEK_CCI_DEVFREQ >>>> + tristate "MEDIATEK CCI DEVFREQ Driver" >>>> + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST >>>> + select DEVFREQ_GOV_PASSIVE >>>> + help >>>> + This adds a devfreq driver for MediaTek Cache Coherent >>>> Interconnect >>>> + which is shared the same regulators with the cpu cluster. It >>>> can track >>>> + buck voltages and update a proper CCI frequency. Use the >>>> notification >>>> + to get the regulator status. >>>> + >>>> config ARM_RK3399_DMC_DEVFREQ >>>> tristate "ARM RK3399 DMC DEVFREQ Driver" >>>> depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ >>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile >>>> index 0b6be92a25d9..bf40d04928d0 100644 >>>> --- a/drivers/devfreq/Makefile >>>> +++ b/drivers/devfreq/Makefile >>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += >>>> governor_passive.o >>>> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o >>>> obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o >>>> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o >>>> +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o >>>> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o >>>> obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- >>>> mbus.o >>>> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o >>>> diff --git a/drivers/devfreq/mtk-cci-devfreq.c >>>> b/drivers/devfreq/mtk- >>>> cci-devfreq.c >>>> new file mode 100644 >>>> index 000000000000..b3e31c45a57c >>>> --- /dev/null >>>> +++ b/drivers/devfreq/mtk-cci-devfreq.c >>>> @@ -0,0 +1,474 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/* >>>> + * Copyright (C) 2022 MediaTek Inc. >>>> + */ >>>> + >>>> +#include <linux/clk.h> >>>> +#include <linux/devfreq.h> >>>> +#include <linux/minmax.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of.h> >>>> +#include <linux/of_device.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/pm_opp.h> >>>> +#include <linux/regulator/consumer.h> >>>> + >>>> +struct mtk_ccifreq_platform_data { >>>> + int min_volt_shift; >>>> + int max_volt_shift; >>>> + int proc_max_volt; >>>> + int sram_min_volt; >>>> + int sram_max_volt; >>>> +}; >>>> + >>>> +struct mtk_ccifreq_drv { >>>> + struct device *dev; >>>> + struct devfreq *devfreq; >>>> + struct regulator *proc_reg; >>>> + struct regulator *sram_reg; >>>> + struct clk *cci_clk; >>>> + struct clk *inter_clk; >>>> + int inter_voltage; >>>> + int pre_voltage; >>>> + unsigned long pre_freq; >>>> + /* Avoid race condition for regulators between notify and >>>> policy */ >>>> + struct mutex reg_lock; >>>> + struct notifier_block opp_nb; >>>> + const struct mtk_ccifreq_platform_data *soc_data; >>>> + int vtrack_max; >>>> +}; >>>> + >>>> +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, >>>> int >>>> new_voltage) >>>> +{ >>>> + const struct mtk_ccifreq_platform_data *soc_data = drv- >>>>> soc_data; >>>> >>>> + struct device *dev = drv->dev; >>>> + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; >>>> + int retry_max = drv->vtrack_max; >>>> + >>>> + if (!drv->sram_reg) { >>>> + ret = regulator_set_voltage(drv->proc_reg, new_voltage, >>>> + drv->soc_data- >>>>> proc_max_volt); >>>> >>>> + goto out_set_voltage; >>>> + } >>>> + >>>> + pre_voltage = regulator_get_voltage(drv->proc_reg); >>>> + if (pre_voltage < 0) { >>>> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >>>> + return pre_voltage; >>>> + } >>>> + >>>> + pre_vsram = regulator_get_voltage(drv->sram_reg); >>>> + if (pre_vsram < 0) { >>>> + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); >>>> + return pre_vsram; >>>> + } >>>> + >>>> + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, >>>> + soc_data->sram_min_volt, soc_data- >>>>> sram_max_volt); >>>> >>>> + >>>> + do { >>>> + if (pre_voltage <= new_voltage) { >>>> + vsram = clamp(pre_voltage + soc_data- >>>>> max_volt_shift, >>>> >>>> + soc_data->sram_min_volt, >>>> new_vsram); >>>> + ret = regulator_set_voltage(drv->sram_reg, >>>> vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (vsram == soc_data->sram_max_volt || >>>> + new_vsram == soc_data->sram_min_volt) >>>> + voltage = new_voltage; >>>> + else >>>> + voltage = vsram - soc_data- >>>>> min_volt_shift; >>>> >>>> + >>>> + ret = regulator_set_voltage(drv->proc_reg, >>>> voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + if (ret) { >>>> + regulator_set_voltage(drv->sram_reg, >>>> pre_vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + return ret; >>>> + } >>>> + } else if (pre_voltage > new_voltage) { >>>> + voltage = max(new_voltage, >>>> + pre_vsram - soc_data- >>>>> max_volt_shift); >>>> >>>> + ret = regulator_set_voltage(drv->proc_reg, >>>> voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (voltage == new_voltage) >>>> + vsram = new_vsram; >>>> + else >>>> + vsram = max(new_vsram, >>>> + voltage + soc_data- >>>>> min_volt_shift); >>>> >>>> + >>>> + ret = regulator_set_voltage(drv->sram_reg, >>>> vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + if (ret) { >>>> + regulator_set_voltage(drv->proc_reg, >>>> pre_voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + pre_voltage = voltage; >>>> + pre_vsram = vsram; >>>> + >>>> + if (--retry_max < 0) { >>>> + dev_err(dev, >>>> + "over loop count, failed to set >>>> voltage\n"); >>>> + return -EINVAL; >>>> + } >>>> + } while (voltage != new_voltage || vsram != new_vsram); >>>> + >>>> +out_set_voltage: >>>> + if (!ret) >>>> + drv->pre_voltage = new_voltage; >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_target(struct device *dev, unsigned long >>>> *freq, >>>> + u32 flags) >>>> +{ >>>> + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); >>>> + struct clk *cci_pll = clk_get_parent(drv->cci_clk); >>>> + struct dev_pm_opp *opp; >>>> + unsigned long opp_rate; >>>> + int voltage, pre_voltage, inter_voltage, target_voltage, ret; >>>> + >>>> + if (!drv) >>>> + return -EINVAL; >>>> + >>>> + if (drv->pre_freq == *freq) >>>> + return 0; >>>> + >>>> + inter_voltage = drv->inter_voltage; >>>> + >>>> + opp_rate = *freq; >>>> + opp = devfreq_recommended_opp(dev, &opp_rate, 1); >>>> + if (IS_ERR(opp)) { >>>> + dev_err(dev, "failed to find opp for freq: %ld\n", >>>> opp_rate); >>>> + return PTR_ERR(opp); >>>> + } >>>> + >>>> + mutex_lock(&drv->reg_lock); >>>> + >>>> + voltage = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + >>>> + if (unlikely(drv->pre_voltage <= 0)) >>>> + pre_voltage = regulator_get_voltage(drv->proc_reg); >>>> + else >>>> + pre_voltage = drv->pre_voltage; >>>> + >>>> + if (pre_voltage < 0) { >>>> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >>>> + return pre_voltage; >>>> + } >>>> + >>>> + /* scale up: set voltage first then freq. */ >>>> + target_voltage = max(inter_voltage, voltage); >>>> + if (pre_voltage <= target_voltage) { >>>> + ret = mtk_ccifreq_set_voltage(drv, target_voltage); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale up voltage\n"); >>>> + goto out_restore_voltage; >>>> + } >>>> + } >>>> + >>>> + /* switch the cci clock to intermediate clock source. */ >>>> + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); >>>> + if (ret) { >>>> + dev_err(dev, "failed to re-parent cci clock\n"); >>>> + goto out_restore_voltage; >>>> + } >>>> + >>>> + /* set the original clock to target rate. */ >>>> + ret = clk_set_rate(cci_pll, *freq); >>>> + if (ret) { >>>> + dev_err(dev, "failed to set cci pll rate: %d\n", ret); >>>> + clk_set_parent(drv->cci_clk, cci_pll); >>>> + goto out_restore_voltage; >>>> + } >>>> + >>>> + /* switch the cci clock back to the original clock source. */ >>>> + ret = clk_set_parent(drv->cci_clk, cci_pll); >>>> + if (ret) { >>>> + dev_err(dev, "failed to re-parent cci clock\n"); >>>> + mtk_ccifreq_set_voltage(drv, inter_voltage); >>>> + goto out_unlock; >>>> + } >>>> + >>>> + /* >>>> + * If the new voltage is lower than the intermediate voltage or >>>> the >>>> + * original voltage, scale down to the new voltage. >>>> + */ >>>> + if (voltage < inter_voltage || voltage < pre_voltage) { >>>> + ret = mtk_ccifreq_set_voltage(drv, voltage); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale down voltage\n"); >>>> + goto out_unlock; >>>> + } >>>> + } >>>> + >>>> + drv->pre_freq = *freq; >>>> + mutex_unlock(&drv->reg_lock); >>>> + >>>> + return 0; >>>> + >>>> +out_restore_voltage: >>>> + mtk_ccifreq_set_voltage(drv, pre_voltage); >>>> + >>>> +out_unlock: >>>> + mutex_unlock(&drv->reg_lock); >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, >>>> + unsigned long event, void *data) >>>> +{ >>>> + struct dev_pm_opp *opp = data; >>>> + struct mtk_ccifreq_drv *drv; >>>> + unsigned long freq, volt; >>>> + >>>> + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); >>>> + >>>> + if (event == OPP_EVENT_ADJUST_VOLTAGE) { >>>> + freq = dev_pm_opp_get_freq(opp); >>>> + >>>> + mutex_lock(&drv->reg_lock); >>>> + /* current opp item is changed */ >>>> + if (freq == drv->pre_freq) { >>>> + volt = dev_pm_opp_get_voltage(opp); >>>> + mtk_ccifreq_set_voltage(drv, volt); >>>> + } >>>> + mutex_unlock(&drv->reg_lock); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static struct devfreq_dev_profile mtk_ccifreq_profile = { >>>> + .target = mtk_ccifreq_target, >>>> +}; >>>> + >>>> +static int mtk_ccifreq_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct mtk_ccifreq_drv *drv; >>>> + struct devfreq_passive_data *passive_data; >>>> + struct dev_pm_opp *opp; >>>> + unsigned long rate, opp_volt; >>>> + int ret; >>>> + >>>> + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); >>>> + if (!drv) >>>> + return -ENOMEM; >>>> + >>>> + drv->dev = dev; >>>> + drv->soc_data = (const struct mtk_ccifreq_platform_data *) >>>> + of_device_get_match_data(&pdev->dev); >>>> + mutex_init(&drv->reg_lock); >>>> + platform_set_drvdata(pdev, drv); >>>> + >>>> + drv->cci_clk = devm_clk_get(dev, "cci"); >>>> + if (IS_ERR(drv->cci_clk)) { >>>> + ret = PTR_ERR(drv->cci_clk); >>>> + return dev_err_probe(dev, ret, >>>> + "failed to get cci clk: %d\n", >>>> ret); >>>> + } >>>> + >>>> + drv->inter_clk = devm_clk_get(dev, "intermediate"); >>>> + if (IS_ERR(drv->inter_clk)) { >>>> + ret = PTR_ERR(drv->inter_clk); >>>> + dev_err_probe(dev, ret, >>>> + "failed to get intermediate clk: %d\n", >>>> ret); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); >>>> + if (IS_ERR(drv->proc_reg)) { >>>> + ret = PTR_ERR(drv->proc_reg); >>>> + dev_err_probe(dev, ret, >>>> + "failed to get proc regulator: %d\n", >>>> ret); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + ret = regulator_enable(drv->proc_reg); >>>> + if (ret) { >>>> + dev_err(dev, "failed to enable proc regulator\n"); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + drv->sram_reg = regulator_get_optional(dev, "sram"); >>>> + if (IS_ERR(drv->sram_reg)) >>>> + drv->sram_reg = NULL; >>>> + else { >>>> + ret = regulator_enable(drv->sram_reg); >>>> + if (ret) { >>>> + dev_err(dev, "failed to enable sram >>>> regulator\n"); >>>> + goto out_free_resources; >>>> + } >>>> + } >>>> + >>>> + /* >>>> + * We assume min voltage is 0 and tracking target voltage using >>>> + * min_volt_shift for each iteration. >>>> + * The retry_max is 3 times of expeted iteration count. >>>> + */ >>>> + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- >>>>> sram_max_volt, >>>> >>>> + drv->soc_data- >>>>> proc_max_volt), >>>> >>>> + drv->soc_data- >>>>> min_volt_shift); >>>> >>>> + >>>> + ret = clk_prepare_enable(drv->cci_clk); >>>> + if (ret) >>>> + goto out_free_resources; >>>> + >>>> + ret = clk_prepare_enable(drv->inter_clk); >>>> + if (ret) >>>> + goto out_disable_cci_clk; >>>> + >>>> + ret = dev_pm_opp_of_add_table(dev); >>>> + if (ret) { >>>> + dev_err(dev, "failed to add opp table: %d\n", ret); >>>> + goto out_disable_inter_clk; >>>> + } >>>> + >>>> + rate = clk_get_rate(drv->inter_clk); >>>> + opp = dev_pm_opp_find_freq_ceil(dev, &rate); >>>> + if (IS_ERR(opp)) { >>>> + ret = PTR_ERR(opp); >>>> + dev_err(dev, "failed to get intermediate opp: %d\n", >>>> ret); >>>> + goto out_remove_opp_table; >>>> + } >>>> + drv->inter_voltage = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + >>>> + rate = U32_MAX; >>>> + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); >>>> + if (IS_ERR(opp)) { >>>> + dev_err(dev, "failed to get opp\n"); >>>> + ret = PTR_ERR(opp); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + opp_volt = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + ret = mtk_ccifreq_set_voltage(drv, opp_volt); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale to highest voltage %lu in >>>> proc_reg\n", >>>> + opp_volt); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + passive_data = devm_kzalloc(dev, sizeof(struct >>>> devfreq_passive_data), >>>> + GFP_KERNEL); >>>> + if (!passive_data) { >>>> + ret = -ENOMEM; >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + passive_data->parent_type = CPUFREQ_PARENT_DEV; >>>> + drv->devfreq = devm_devfreq_add_device(dev, >>>> &mtk_ccifreq_profile, >>>> + DEVFREQ_GOV_PASSIVE, >>>> + passive_data); >>>> + if (IS_ERR(drv->devfreq)) { >>>> + ret = -EPROBE_DEFER; >>>> + dev_err(dev, "failed to add devfreq device: %d\n", >>>> + PTR_ERR(drv->devfreq)); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; >>>> + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); >>>> + if (ret) { >>>> + dev_err(dev, "failed to register opp notifier: %d\n", >>>> ret); >>>> + goto out_remove_devfreq_device; >>>> + } >>>> + return 0; >>>> + >>>> +out_remove_devfreq_device: >>>> + devm_devfreq_remove_device(dev, drv->devfreq); >>>> + >>>> +out_remove_opp_table: >>>> + dev_pm_opp_of_remove_table(dev); >>>> + >>>> +out_disable_inter_clk: >>>> + clk_disable_unprepare(drv->inter_clk); >>>> + >>>> +out_disable_cci_clk: >>>> + clk_disable_unprepare(drv->cci_clk); >>>> + >>>> +out_free_resources: >>>> + if (regulator_is_enabled(drv->proc_reg)) >>>> + regulator_disable(drv->proc_reg); >>>> + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) >>>> + regulator_disable(drv->sram_reg); >>>> + >>>> + if (!IS_ERR(drv->proc_reg)) >>>> + regulator_put(drv->proc_reg); >>>> + if (!IS_ERR(drv->sram_reg)) >>>> + regulator_put(drv->sram_reg); >>>> + if (!IS_ERR(drv->cci_clk)) >>>> + clk_put(drv->cci_clk); >>>> + if (!IS_ERR(drv->inter_clk)) >>>> + clk_put(drv->inter_clk); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_remove(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct mtk_ccifreq_drv *drv; >>>> + >>>> + drv = platform_get_drvdata(pdev); >>>> + >>>> + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); >>>> + dev_pm_opp_of_remove_table(dev); >>>> + clk_disable_unprepare(drv->inter_clk); >>>> + clk_disable_unprepare(drv->cci_clk); >>>> + regulator_disable(drv->proc_reg); >>>> + if (drv->sram_reg) >>>> + regulator_disable(drv->sram_reg); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static const struct mtk_ccifreq_platform_data >>>> mt8183_platform_data = >>>> { >>>> + .min_volt_shift = 100000, >>>> + .max_volt_shift = 200000, >>>> + .proc_max_volt = 1150000, >>>> + .sram_min_volt = 0, >>>> + .sram_max_volt = 1150000, >>>> +}; >>>> + >>>> +static const struct mtk_ccifreq_platform_data >>>> mt8186_platform_data = >>>> { >>>> + .min_volt_shift = 100000, >>>> + .max_volt_shift = 250000, >>>> + .proc_max_volt = 1118750, >>>> + .sram_min_volt = 850000, >>>> + .sram_max_volt = 1118750, >>>> +}; >>>> + >>>> +static const struct of_device_id mtk_ccifreq_machines[] = { >>>> + { .compatible = "mediatek,mt8183-cci", .data = >>>> &mt8183_platform_data }, >>>> + { .compatible = "mediatek,mt8186-cci", .data = >>>> &mt8186_platform_data }, >>>> + { }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); >>>> + >>>> +static struct platform_driver mtk_ccifreq_platdrv = { >>>> + .probe = mtk_ccifreq_probe, >>>> + .remove = mtk_ccifreq_remove, >>>> + .driver = { >>>> + .name = "mtk-ccifreq", >>>> + .of_match_table = mtk_ccifreq_machines, >>>> + }, >>>> +}; >>>> +module_platform_driver(mtk_ccifreq_platdrv); >>>> + >>>> +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); >>>> +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); >>>> +MODULE_LICENSE("GPL v2"); >>> >>> Hi Chanwoo, >>> >>> Just a kindly ping. >>> Could you please give me some suggestion on this patch? >>> Thanks! >>> >>> BRs, >>> Johnson Wang >>> >> >> Hi Johnson, >> >> Sorry for late reply.But I replied it yesterday as following: >> Maybe, this reply[1] has not yet arrrived at your mail box. >> [1] >> > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef >> >> As I described on reply[1], I updated the patches on devfreq-testing >> branch. Could you please test your patches based on devfreq-testing >> branch? >> > > Hi Chanwoo, > > Thanks for your information. > I've tested this patch based on the latest devfreq-testing branch. > It encounters the same crash as Chen-Yu mentioned[1]. > > [1]https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u Hi Johnson, Thanks for the test. I'll drop the last patch caused of crash. And I'll send v3 patchset right now. -- Best Regards, Samsung Electronics Chanwoo Choi
On Mon, 2022-05-09 at 20:51 +0900, Chanwoo Choi wrote: > On 22. 5. 9. 14:57, Johnson Wang wrote: > > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > > > On 22. 5. 6. 20:38, Johnson Wang wrote: > > > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > > > Interconnect > > > > > (CCI) used by some MediaTek SoCs. > > > > > > > > > > In this driver, we use the passive devfreq driver to get > > > > > target > > > > > frequencies > > > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > > > MediaTek > > > > > CCI > > > > > is supplied by the same regulators with the little core CPUs. > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > > --- > > > > > This patch depends on "devfreq-testing"[1]. > > > > > [1] > > > > > > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > > > > > --- > > > > > drivers/devfreq/Kconfig | 10 + > > > > > drivers/devfreq/Makefile | 1 + > > > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > > > ++++++++++++++++++++++++++++++ > > > > > 3 files changed, 485 insertions(+) > > > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > > > > > diff --git a/drivers/devfreq/Kconfig > > > > > b/drivers/devfreq/Kconfig > > > > > index 87eb2b837e68..9754d8b31621 100644 > > > > > --- a/drivers/devfreq/Kconfig > > > > > +++ b/drivers/devfreq/Kconfig > > > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > > > It reads ACTMON counters of memory controllers and > > > > > adjusts > > > > > the > > > > > operating frequencies and voltages with OPP support. > > > > > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > > > + select DEVFREQ_GOV_PASSIVE > > > > > + help > > > > > + This adds a devfreq driver for MediaTek Cache > > > > > Coherent > > > > > Interconnect > > > > > + which is shared the same regulators with the cpu > > > > > cluster. It > > > > > can track > > > > > + buck voltages and update a proper CCI frequency. Use > > > > > the > > > > > notification > > > > > + to get the regulator status. > > > > > + > > > > > config ARM_RK3399_DMC_DEVFREQ > > > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > > > diff --git a/drivers/devfreq/Makefile > > > > > b/drivers/devfreq/Makefile > > > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > > > --- a/drivers/devfreq/Makefile > > > > > +++ b/drivers/devfreq/Makefile > > > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > > > governor_passive.o > > > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci- > > > > > devfreq.o > > > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > > > mbus.o > > > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30- > > > > > devfreq.o > > > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > > > b/drivers/devfreq/mtk- > > > > > cci-devfreq.c > > > > > new file mode 100644 > > > > > index 000000000000..b3e31c45a57c > > > > > --- /dev/null > > > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > > > @@ -0,0 +1,474 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > +/* > > > > > + * Copyright (C) 2022 MediaTek Inc. > > > > > + */ > > > > > + > > > > > +#include <linux/clk.h> > > > > > +#include <linux/devfreq.h> > > > > > +#include <linux/minmax.h> > > > > > +#include <linux/module.h> > > > > > +#include <linux/of.h> > > > > > +#include <linux/of_device.h> > > > > > +#include <linux/platform_device.h> > > > > > +#include <linux/pm_opp.h> > > > > > +#include <linux/regulator/consumer.h> > > > > > + > > > > > +struct mtk_ccifreq_platform_data { > > > > > + int min_volt_shift; > > > > > + int max_volt_shift; > > > > > + int proc_max_volt; > > > > > + int sram_min_volt; > > > > > + int sram_max_volt; > > > > > +}; > > > > > + > > > > > +struct mtk_ccifreq_drv { > > > > > + struct device *dev; > > > > > + struct devfreq *devfreq; > > > > > + struct regulator *proc_reg; > > > > > + struct regulator *sram_reg; > > > > > + struct clk *cci_clk; > > > > > + struct clk *inter_clk; > > > > > + int inter_voltage; > > > > > + int pre_voltage; > > > > > + unsigned long pre_freq; > > > > > + /* Avoid race condition for regulators between notify > > > > > and > > > > > policy */ > > > > > + struct mutex reg_lock; > > > > > + struct notifier_block opp_nb; > > > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > > > + int vtrack_max; > > > > > +}; > > > > > + > > > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv > > > > > *drv, > > > > > int > > > > > new_voltage) > > > > > +{ > > > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > > > soc_data; > > > > > > > > > > + struct device *dev = drv->dev; > > > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, > > > > > ret; > > > > > + int retry_max = drv->vtrack_max; > > > > > + > > > > > + if (!drv->sram_reg) { > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > > > new_voltage, > > > > > + drv->soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + goto out_set_voltage; > > > > > + } > > > > > + > > > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > > > + if (pre_voltage < 0) { > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > pre_voltage); > > > > > + return pre_voltage; > > > > > + } > > > > > + > > > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > > > + if (pre_vsram < 0) { > > > > > + dev_err(dev, "invalid vsram value: %d\n", > > > > > pre_vsram); > > > > > + return pre_vsram; > > > > > + } > > > > > + > > > > > + new_vsram = clamp(new_voltage + soc_data- > > > > > >min_volt_shift, > > > > > + soc_data->sram_min_volt, soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + > > > > > + do { > > > > > + if (pre_voltage <= new_voltage) { > > > > > + vsram = clamp(pre_voltage + soc_data- > > > > > > max_volt_shift, > > > > > > > > > > + soc_data->sram_min_volt, > > > > > new_vsram); > > > > > + ret = regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + if (vsram == soc_data->sram_max_volt || > > > > > + new_vsram == soc_data- > > > > > >sram_min_volt) > > > > > + voltage = new_voltage; > > > > > + else > > > > > + voltage = vsram - soc_data- > > > > > > min_volt_shift; > > > > > > > > > > + > > > > > + ret = regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + if (ret) { > > > > > + regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > pre_vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + return ret; > > > > > + } > > > > > + } else if (pre_voltage > new_voltage) { > > > > > + voltage = max(new_voltage, > > > > > + pre_vsram - soc_data- > > > > > > max_volt_shift); > > > > > > > > > > + ret = regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + if (voltage == new_voltage) > > > > > + vsram = new_vsram; > > > > > + else > > > > > + vsram = max(new_vsram, > > > > > + voltage + soc_data- > > > > > > min_volt_shift); > > > > > > > > > > + > > > > > + ret = regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + if (ret) { > > > > > + regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > pre_voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + return ret; > > > > > + } > > > > > + } > > > > > + > > > > > + pre_voltage = voltage; > > > > > + pre_vsram = vsram; > > > > > + > > > > > + if (--retry_max < 0) { > > > > > + dev_err(dev, > > > > > + "over loop count, failed to set > > > > > voltage\n"); > > > > > + return -EINVAL; > > > > > + } > > > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > > > + > > > > > +out_set_voltage: > > > > > + if (!ret) > > > > > + drv->pre_voltage = new_voltage; > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_target(struct device *dev, unsigned > > > > > long > > > > > *freq, > > > > > + u32 flags) > > > > > +{ > > > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > > > + struct dev_pm_opp *opp; > > > > > + unsigned long opp_rate; > > > > > + int voltage, pre_voltage, inter_voltage, > > > > > target_voltage, ret; > > > > > + > > > > > + if (!drv) > > > > > + return -EINVAL; > > > > > + > > > > > + if (drv->pre_freq == *freq) > > > > > + return 0; > > > > > + > > > > > + inter_voltage = drv->inter_voltage; > > > > > + > > > > > + opp_rate = *freq; > > > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > > > + if (IS_ERR(opp)) { > > > > > + dev_err(dev, "failed to find opp for freq: > > > > > %ld\n", > > > > > opp_rate); > > > > > + return PTR_ERR(opp); > > > > > + } > > > > > + > > > > > + mutex_lock(&drv->reg_lock); > > > > > + > > > > > + voltage = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + > > > > > + if (unlikely(drv->pre_voltage <= 0)) > > > > > + pre_voltage = regulator_get_voltage(drv- > > > > > >proc_reg); > > > > > + else > > > > > + pre_voltage = drv->pre_voltage; > > > > > + > > > > > + if (pre_voltage < 0) { > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > pre_voltage); > > > > > + return pre_voltage; > > > > > + } > > > > > + > > > > > + /* scale up: set voltage first then freq. */ > > > > > + target_voltage = max(inter_voltage, voltage); > > > > > + if (pre_voltage <= target_voltage) { > > > > > + ret = mtk_ccifreq_set_voltage(drv, > > > > > target_voltage); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale up > > > > > voltage\n"); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + } > > > > > + > > > > > + /* switch the cci clock to intermediate clock source. > > > > > */ > > > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to re-parent cci > > > > > clock\n"); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + > > > > > + /* set the original clock to target rate. */ > > > > > + ret = clk_set_rate(cci_pll, *freq); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to set cci pll rate: > > > > > %d\n", ret); > > > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + > > > > > + /* switch the cci clock back to the original clock > > > > > source. */ > > > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to re-parent cci > > > > > clock\n"); > > > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > > > + goto out_unlock; > > > > > + } > > > > > + > > > > > + /* > > > > > + * If the new voltage is lower than the intermediate > > > > > voltage or > > > > > the > > > > > + * original voltage, scale down to the new voltage. > > > > > + */ > > > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale down > > > > > voltage\n"); > > > > > + goto out_unlock; > > > > > + } > > > > > + } > > > > > + > > > > > + drv->pre_freq = *freq; > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + > > > > > + return 0; > > > > > + > > > > > +out_restore_voltage: > > > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > > > + > > > > > +out_unlock: > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block > > > > > *nb, > > > > > + unsigned long event, void > > > > > *data) > > > > > +{ > > > > > + struct dev_pm_opp *opp = data; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + unsigned long freq, volt; > > > > > + > > > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > > > + > > > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > > > + freq = dev_pm_opp_get_freq(opp); > > > > > + > > > > > + mutex_lock(&drv->reg_lock); > > > > > + /* current opp item is changed */ > > > > > + if (freq == drv->pre_freq) { > > > > > + volt = dev_pm_opp_get_voltage(opp); > > > > > + mtk_ccifreq_set_voltage(drv, volt); > > > > > + } > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + } > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > > > + .target = mtk_ccifreq_target, > > > > > +}; > > > > > + > > > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > > > +{ > > > > > + struct device *dev = &pdev->dev; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + struct devfreq_passive_data *passive_data; > > > > > + struct dev_pm_opp *opp; > > > > > + unsigned long rate, opp_volt; > > > > > + int ret; > > > > > + > > > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > > > + if (!drv) > > > > > + return -ENOMEM; > > > > > + > > > > > + drv->dev = dev; > > > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data > > > > > *) > > > > > + of_device_get_match_data(&pdev- > > > > > >dev); > > > > > + mutex_init(&drv->reg_lock); > > > > > + platform_set_drvdata(pdev, drv); > > > > > + > > > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > > > + if (IS_ERR(drv->cci_clk)) { > > > > > + ret = PTR_ERR(drv->cci_clk); > > > > > + return dev_err_probe(dev, ret, > > > > > + "failed to get cci clk: > > > > > %d\n", > > > > > ret); > > > > > + } > > > > > + > > > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > > > + if (IS_ERR(drv->inter_clk)) { > > > > > + ret = PTR_ERR(drv->inter_clk); > > > > > + dev_err_probe(dev, ret, > > > > > + "failed to get intermediate clk: > > > > > %d\n", > > > > > ret); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + drv->proc_reg = devm_regulator_get_optional(dev, > > > > > "proc"); > > > > > + if (IS_ERR(drv->proc_reg)) { > > > > > + ret = PTR_ERR(drv->proc_reg); > > > > > + dev_err_probe(dev, ret, > > > > > + "failed to get proc regulator: > > > > > %d\n", > > > > > ret); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + ret = regulator_enable(drv->proc_reg); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to enable proc > > > > > regulator\n"); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > > > + if (IS_ERR(drv->sram_reg)) > > > > > + drv->sram_reg = NULL; > > > > > + else { > > > > > + ret = regulator_enable(drv->sram_reg); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to enable sram > > > > > regulator\n"); > > > > > + goto out_free_resources; > > > > > + } > > > > > + } > > > > > + > > > > > + /* > > > > > + * We assume min voltage is 0 and tracking target > > > > > voltage using > > > > > + * min_volt_shift for each iteration. > > > > > + * The retry_max is 3 times of expeted iteration count. > > > > > + */ > > > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > > > sram_max_volt, > > > > > > > > > > + drv->soc_data- > > > > > > proc_max_volt), > > > > > > > > > > + drv->soc_data- > > > > > > min_volt_shift); > > > > > > > > > > + > > > > > + ret = clk_prepare_enable(drv->cci_clk); > > > > > + if (ret) > > > > > + goto out_free_resources; > > > > > + > > > > > + ret = clk_prepare_enable(drv->inter_clk); > > > > > + if (ret) > > > > > + goto out_disable_cci_clk; > > > > > + > > > > > + ret = dev_pm_opp_of_add_table(dev); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to add opp table: %d\n", > > > > > ret); > > > > > + goto out_disable_inter_clk; > > > > > + } > > > > > + > > > > > + rate = clk_get_rate(drv->inter_clk); > > > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > > > + if (IS_ERR(opp)) { > > > > > + ret = PTR_ERR(opp); > > > > > + dev_err(dev, "failed to get intermediate opp: > > > > > %d\n", > > > > > ret); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + > > > > > + rate = U32_MAX; > > > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > > > + if (IS_ERR(opp)) { > > > > > + dev_err(dev, "failed to get opp\n"); > > > > > + ret = PTR_ERR(opp); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale to highest > > > > > voltage %lu in > > > > > proc_reg\n", > > > > > + opp_volt); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > > > devfreq_passive_data), > > > > > + GFP_KERNEL); > > > > > + if (!passive_data) { > > > > > + ret = -ENOMEM; > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > > > &mtk_ccifreq_profile, > > > > > + DEVFREQ_GOV_PASS > > > > > IVE, > > > > > + passive_data); > > > > > + if (IS_ERR(drv->devfreq)) { > > > > > + ret = -EPROBE_DEFER; > > > > > + dev_err(dev, "failed to add devfreq device: > > > > > %d\n", > > > > > + PTR_ERR(drv->devfreq)); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to register opp notifier: > > > > > %d\n", > > > > > ret); > > > > > + goto out_remove_devfreq_device; > > > > > + } > > > > > + return 0; > > > > > + > > > > > +out_remove_devfreq_device: > > > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > > > + > > > > > +out_remove_opp_table: > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > + > > > > > +out_disable_inter_clk: > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > + > > > > > +out_disable_cci_clk: > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > + > > > > > +out_free_resources: > > > > > + if (regulator_is_enabled(drv->proc_reg)) > > > > > + regulator_disable(drv->proc_reg); > > > > > + if (drv->sram_reg && regulator_is_enabled(drv- > > > > > >sram_reg)) > > > > > + regulator_disable(drv->sram_reg); > > > > > + > > > > > + if (!IS_ERR(drv->proc_reg)) > > > > > + regulator_put(drv->proc_reg); > > > > > + if (!IS_ERR(drv->sram_reg)) > > > > > + regulator_put(drv->sram_reg); > > > > > + if (!IS_ERR(drv->cci_clk)) > > > > > + clk_put(drv->cci_clk); > > > > > + if (!IS_ERR(drv->inter_clk)) > > > > > + clk_put(drv->inter_clk); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > > > +{ > > > > > + struct device *dev = &pdev->dev; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + > > > > > + drv = platform_get_drvdata(pdev); > > > > > + > > > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > + regulator_disable(drv->proc_reg); > > > > > + if (drv->sram_reg) > > > > > + regulator_disable(drv->sram_reg); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static const struct mtk_ccifreq_platform_data > > > > > mt8183_platform_data = > > > > > { > > > > > + .min_volt_shift = 100000, > > > > > + .max_volt_shift = 200000, > > > > > + .proc_max_volt = 1150000, > > > > > + .sram_min_volt = 0, > > > > > + .sram_max_volt = 1150000, > > > > > +}; > > > > > + > > > > > +static const struct mtk_ccifreq_platform_data > > > > > mt8186_platform_data = > > > > > { > > > > > + .min_volt_shift = 100000, > > > > > + .max_volt_shift = 250000, > > > > > + .proc_max_volt = 1118750, > > > > > + .sram_min_volt = 850000, > > > > > + .sram_max_volt = 1118750, > > > > > +}; > > > > > + > > > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > > > &mt8183_platform_data }, > > > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > > > &mt8186_platform_data }, > > > > > + { }, > > > > > +}; > > > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > > > + > > > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > > > + .probe = mtk_ccifreq_probe, > > > > > + .remove = mtk_ccifreq_remove, > > > > > + .driver = { > > > > > + .name = "mtk-ccifreq", > > > > > + .of_match_table = mtk_ccifreq_machines, > > > > > + }, > > > > > +}; > > > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > > > + > > > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > Hi Chanwoo, > > > > > > > > Just a kindly ping. > > > > Could you please give me some suggestion on this patch? > > > > Thanks! > > > > > > > > BRs, > > > > Johnson Wang > > > > > > > > > > Hi Johnson, > > > > > > Sorry for late reply.But I replied it yesterday as following: > > > Maybe, this reply[1] has not yet arrrived at your mail box. > > > [1] > > > > > > > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > > > > > As I described on reply[1], I updated the patches on devfreq- > > > testing > > > branch. Could you please test your patches based on devfreq- > > > testing > > > branch? > > > > > > > Hi Chanwoo, > > > > Thanks for your information. > > I've tested this patch based on the latest devfreq-testing branch. > > It encounters the same crash as Chen-Yu mentioned[1]. > > > > [1] > > https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u > > Hi Johnson, > > Thanks for the test. I'll drop the last patch caused of crash. > And I'll send v3 patchset right now. > > Hi Chanwoo, With your v3 patchset, this CCI devfreq driver works properly on the target. Thanks! BRs, Johnson Wang
On Wed, May 11, 2022 at 2:14 PM Johnson Wang <johnson.wang@mediatek.com> wrote: > > On Mon, 2022-05-09 at 20:51 +0900, Chanwoo Choi wrote: > > On 22. 5. 9. 14:57, Johnson Wang wrote: > > > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > > > > On 22. 5. 6. 20:38, Johnson Wang wrote: > > > > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > > > > Interconnect > > > > > > (CCI) used by some MediaTek SoCs. > > > > > > > > > > > > In this driver, we use the passive devfreq driver to get > > > > > > target > > > > > > frequencies > > > > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > > > > MediaTek > > > > > > CCI > > > > > > is supplied by the same regulators with the little core CPUs. > > > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > > > --- > > > > > > This patch depends on "devfreq-testing"[1]. > > > > > > [1] > > > > > > > > > > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > > > > > > > --- > > > > > > drivers/devfreq/Kconfig | 10 + > > > > > > drivers/devfreq/Makefile | 1 + > > > > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > > > > ++++++++++++++++++++++++++++++ > > > > > > 3 files changed, 485 insertions(+) > > > > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > > > > > > > diff --git a/drivers/devfreq/Kconfig > > > > > > b/drivers/devfreq/Kconfig > > > > > > index 87eb2b837e68..9754d8b31621 100644 > > > > > > --- a/drivers/devfreq/Kconfig > > > > > > +++ b/drivers/devfreq/Kconfig > > > > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > > > > It reads ACTMON counters of memory controllers and > > > > > > adjusts > > > > > > the > > > > > > operating frequencies and voltages with OPP support. > > > > > > > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > > > > + select DEVFREQ_GOV_PASSIVE > > > > > > + help > > > > > > + This adds a devfreq driver for MediaTek Cache > > > > > > Coherent > > > > > > Interconnect > > > > > > + which is shared the same regulators with the cpu > > > > > > cluster. It > > > > > > can track > > > > > > + buck voltages and update a proper CCI frequency. Use > > > > > > the > > > > > > notification > > > > > > + to get the regulator status. > > > > > > + > > > > > > config ARM_RK3399_DMC_DEVFREQ > > > > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > > > > diff --git a/drivers/devfreq/Makefile > > > > > > b/drivers/devfreq/Makefile > > > > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > > > > --- a/drivers/devfreq/Makefile > > > > > > +++ b/drivers/devfreq/Makefile > > > > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > > > > governor_passive.o > > > > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci- > > > > > > devfreq.o > > > > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > > > > mbus.o > > > > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30- > > > > > > devfreq.o > > > > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > > > > b/drivers/devfreq/mtk- > > > > > > cci-devfreq.c > > > > > > new file mode 100644 > > > > > > index 000000000000..b3e31c45a57c > > > > > > --- /dev/null > > > > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > > > > @@ -0,0 +1,474 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > > +/* > > > > > > + * Copyright (C) 2022 MediaTek Inc. > > > > > > + */ > > > > > > + > > > > > > +#include <linux/clk.h> > > > > > > +#include <linux/devfreq.h> > > > > > > +#include <linux/minmax.h> > > > > > > +#include <linux/module.h> > > > > > > +#include <linux/of.h> > > > > > > +#include <linux/of_device.h> > > > > > > +#include <linux/platform_device.h> > > > > > > +#include <linux/pm_opp.h> > > > > > > +#include <linux/regulator/consumer.h> > > > > > > + > > > > > > +struct mtk_ccifreq_platform_data { > > > > > > + int min_volt_shift; > > > > > > + int max_volt_shift; > > > > > > + int proc_max_volt; > > > > > > + int sram_min_volt; > > > > > > + int sram_max_volt; > > > > > > +}; > > > > > > + > > > > > > +struct mtk_ccifreq_drv { > > > > > > + struct device *dev; > > > > > > + struct devfreq *devfreq; > > > > > > + struct regulator *proc_reg; > > > > > > + struct regulator *sram_reg; > > > > > > + struct clk *cci_clk; > > > > > > + struct clk *inter_clk; > > > > > > + int inter_voltage; > > > > > > + int pre_voltage; > > > > > > + unsigned long pre_freq; > > > > > > + /* Avoid race condition for regulators between notify > > > > > > and > > > > > > policy */ > > > > > > + struct mutex reg_lock; > > > > > > + struct notifier_block opp_nb; > > > > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > > > > + int vtrack_max; > > > > > > +}; > > > > > > + > > > > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv > > > > > > *drv, > > > > > > int > > > > > > new_voltage) > > > > > > +{ > > > > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > > > > soc_data; > > > > > > > > > > > > + struct device *dev = drv->dev; > > > > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, > > > > > > ret; > > > > > > + int retry_max = drv->vtrack_max; > > > > > > + > > > > > > + if (!drv->sram_reg) { > > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > > > > new_voltage, > > > > > > + drv->soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + goto out_set_voltage; > > > > > > + } > > > > > > + > > > > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > > > > + if (pre_voltage < 0) { > > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > > pre_voltage); > > > > > > + return pre_voltage; > > > > > > + } > > > > > > + > > > > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > > > > + if (pre_vsram < 0) { > > > > > > + dev_err(dev, "invalid vsram value: %d\n", > > > > > > pre_vsram); > > > > > > + return pre_vsram; > > > > > > + } > > > > > > + > > > > > > + new_vsram = clamp(new_voltage + soc_data- > > > > > > >min_volt_shift, > > > > > > + soc_data->sram_min_volt, soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + > > > > > > + do { > > > > > > + if (pre_voltage <= new_voltage) { > > > > > > + vsram = clamp(pre_voltage + soc_data- > > > > > > > max_volt_shift, > > > > > > > > > > > > + soc_data->sram_min_volt, > > > > > > new_vsram); > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + if (vsram == soc_data->sram_max_volt || > > > > > > + new_vsram == soc_data- > > > > > > >sram_min_volt) > > > > > > + voltage = new_voltage; > > > > > > + else > > > > > > + voltage = vsram - soc_data- > > > > > > > min_volt_shift; > > > > > > > > > > > > + > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + if (ret) { > > > > > > + regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > pre_vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + return ret; > > > > > > + } > > > > > > + } else if (pre_voltage > new_voltage) { > > > > > > + voltage = max(new_voltage, > > > > > > + pre_vsram - soc_data- > > > > > > > max_volt_shift); > > > > > > > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + if (voltage == new_voltage) > > > > > > + vsram = new_vsram; > > > > > > + else > > > > > > + vsram = max(new_vsram, > > > > > > + voltage + soc_data- > > > > > > > min_volt_shift); > > > > > > > > > > > > + > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + if (ret) { > > > > > > + regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > pre_voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + return ret; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + pre_voltage = voltage; > > > > > > + pre_vsram = vsram; > > > > > > + > > > > > > + if (--retry_max < 0) { > > > > > > + dev_err(dev, > > > > > > + "over loop count, failed to set > > > > > > voltage\n"); > > > > > > + return -EINVAL; > > > > > > + } > > > > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > > > > + > > > > > > +out_set_voltage: > > > > > > + if (!ret) > > > > > > + drv->pre_voltage = new_voltage; > > > > > > + > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_target(struct device *dev, unsigned > > > > > > long > > > > > > *freq, > > > > > > + u32 flags) > > > > > > +{ > > > > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > > > > + struct dev_pm_opp *opp; > > > > > > + unsigned long opp_rate; > > > > > > + int voltage, pre_voltage, inter_voltage, > > > > > > target_voltage, ret; > > > > > > + > > > > > > + if (!drv) > > > > > > + return -EINVAL; > > > > > > + > > > > > > + if (drv->pre_freq == *freq) > > > > > > + return 0; > > > > > > + > > > > > > + inter_voltage = drv->inter_voltage; > > > > > > + > > > > > > + opp_rate = *freq; > > > > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > > > > + if (IS_ERR(opp)) { > > > > > > + dev_err(dev, "failed to find opp for freq: > > > > > > %ld\n", > > > > > > opp_rate); > > > > > > + return PTR_ERR(opp); > > > > > > + } > > > > > > + > > > > > > + mutex_lock(&drv->reg_lock); > > > > > > + > > > > > > + voltage = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + > > > > > > + if (unlikely(drv->pre_voltage <= 0)) > > > > > > + pre_voltage = regulator_get_voltage(drv- > > > > > > >proc_reg); > > > > > > + else > > > > > > + pre_voltage = drv->pre_voltage; > > > > > > + > > > > > > + if (pre_voltage < 0) { > > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > > pre_voltage); > > > > > > + return pre_voltage; > > > > > > + } > > > > > > + > > > > > > + /* scale up: set voltage first then freq. */ > > > > > > + target_voltage = max(inter_voltage, voltage); > > > > > > + if (pre_voltage <= target_voltage) { > > > > > > + ret = mtk_ccifreq_set_voltage(drv, > > > > > > target_voltage); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale up > > > > > > voltage\n"); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + /* switch the cci clock to intermediate clock source. > > > > > > */ > > > > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to re-parent cci > > > > > > clock\n"); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + > > > > > > + /* set the original clock to target rate. */ > > > > > > + ret = clk_set_rate(cci_pll, *freq); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to set cci pll rate: > > > > > > %d\n", ret); > > > > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + > > > > > > + /* switch the cci clock back to the original clock > > > > > > source. */ > > > > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to re-parent cci > > > > > > clock\n"); > > > > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > > > > + goto out_unlock; > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * If the new voltage is lower than the intermediate > > > > > > voltage or > > > > > > the > > > > > > + * original voltage, scale down to the new voltage. > > > > > > + */ > > > > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale down > > > > > > voltage\n"); > > > > > > + goto out_unlock; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + drv->pre_freq = *freq; > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + > > > > > > + return 0; > > > > > > + > > > > > > +out_restore_voltage: > > > > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > > > > + > > > > > > +out_unlock: > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block > > > > > > *nb, > > > > > > + unsigned long event, void > > > > > > *data) > > > > > > +{ > > > > > > + struct dev_pm_opp *opp = data; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + unsigned long freq, volt; > > > > > > + > > > > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > > > > + > > > > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > > > > + freq = dev_pm_opp_get_freq(opp); > > > > > > + > > > > > > + mutex_lock(&drv->reg_lock); > > > > > > + /* current opp item is changed */ > > > > > > + if (freq == drv->pre_freq) { > > > > > > + volt = dev_pm_opp_get_voltage(opp); > > > > > > + mtk_ccifreq_set_voltage(drv, volt); > > > > > > + } > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + } > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > > > > + .target = mtk_ccifreq_target, > > > > > > +}; > > > > > > + > > > > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > > > > +{ > > > > > > + struct device *dev = &pdev->dev; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + struct devfreq_passive_data *passive_data; > > > > > > + struct dev_pm_opp *opp; > > > > > > + unsigned long rate, opp_volt; > > > > > > + int ret; > > > > > > + > > > > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > > > > + if (!drv) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + drv->dev = dev; > > > > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data > > > > > > *) > > > > > > + of_device_get_match_data(&pdev- > > > > > > >dev); > > > > > > + mutex_init(&drv->reg_lock); > > > > > > + platform_set_drvdata(pdev, drv); > > > > > > + > > > > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > > > > + if (IS_ERR(drv->cci_clk)) { > > > > > > + ret = PTR_ERR(drv->cci_clk); > > > > > > + return dev_err_probe(dev, ret, > > > > > > + "failed to get cci clk: > > > > > > %d\n", > > > > > > ret); > > > > > > + } > > > > > > + > > > > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > > > > + if (IS_ERR(drv->inter_clk)) { > > > > > > + ret = PTR_ERR(drv->inter_clk); > > > > > > + dev_err_probe(dev, ret, > > > > > > + "failed to get intermediate clk: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + drv->proc_reg = devm_regulator_get_optional(dev, > > > > > > "proc"); > > > > > > + if (IS_ERR(drv->proc_reg)) { > > > > > > + ret = PTR_ERR(drv->proc_reg); > > > > > > + dev_err_probe(dev, ret, > > > > > > + "failed to get proc regulator: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + ret = regulator_enable(drv->proc_reg); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to enable proc > > > > > > regulator\n"); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > > > > + if (IS_ERR(drv->sram_reg)) > > > > > > + drv->sram_reg = NULL; > > > > > > + else { > > > > > > + ret = regulator_enable(drv->sram_reg); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to enable sram > > > > > > regulator\n"); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * We assume min voltage is 0 and tracking target > > > > > > voltage using > > > > > > + * min_volt_shift for each iteration. > > > > > > + * The retry_max is 3 times of expeted iteration count. > > > > > > + */ > > > > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > > > > sram_max_volt, > > > > > > > > > > > > + drv->soc_data- > > > > > > > proc_max_volt), > > > > > > > > > > > > + drv->soc_data- > > > > > > > min_volt_shift); > > > > > > > > > > > > + > > > > > > + ret = clk_prepare_enable(drv->cci_clk); > > > > > > + if (ret) > > > > > > + goto out_free_resources; > > > > > > + > > > > > > + ret = clk_prepare_enable(drv->inter_clk); > > > > > > + if (ret) > > > > > > + goto out_disable_cci_clk; > > > > > > + > > > > > > + ret = dev_pm_opp_of_add_table(dev); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to add opp table: %d\n", > > > > > > ret); > > > > > > + goto out_disable_inter_clk; > > > > > > + } > > > > > > + > > > > > > + rate = clk_get_rate(drv->inter_clk); > > > > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > > > > + if (IS_ERR(opp)) { > > > > > > + ret = PTR_ERR(opp); > > > > > > + dev_err(dev, "failed to get intermediate opp: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + > > > > > > + rate = U32_MAX; > > > > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > > > > + if (IS_ERR(opp)) { > > > > > > + dev_err(dev, "failed to get opp\n"); > > > > > > + ret = PTR_ERR(opp); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale to highest > > > > > > voltage %lu in > > > > > > proc_reg\n", > > > > > > + opp_volt); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > > > > devfreq_passive_data), > > > > > > + GFP_KERNEL); > > > > > > + if (!passive_data) { > > > > > > + ret = -ENOMEM; > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > > > > &mtk_ccifreq_profile, > > > > > > + DEVFREQ_GOV_PASS > > > > > > IVE, > > > > > > + passive_data); > > > > > > + if (IS_ERR(drv->devfreq)) { > > > > > > + ret = -EPROBE_DEFER; > > > > > > + dev_err(dev, "failed to add devfreq device: > > > > > > %d\n", > > > > > > + PTR_ERR(drv->devfreq)); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to register opp notifier: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_remove_devfreq_device; > > > > > > + } > > > > > > + return 0; > > > > > > + > > > > > > +out_remove_devfreq_device: > > > > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > > > > + > > > > > > +out_remove_opp_table: > > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > > + > > > > > > +out_disable_inter_clk: > > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > > + > > > > > > +out_disable_cci_clk: > > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > > + > > > > > > +out_free_resources: > > > > > > + if (regulator_is_enabled(drv->proc_reg)) > > > > > > + regulator_disable(drv->proc_reg); > > > > > > + if (drv->sram_reg && regulator_is_enabled(drv- > > > > > > >sram_reg)) > > > > > > + regulator_disable(drv->sram_reg); > > > > > > + > > > > > > + if (!IS_ERR(drv->proc_reg)) > > > > > > + regulator_put(drv->proc_reg); > > > > > > + if (!IS_ERR(drv->sram_reg)) > > > > > > + regulator_put(drv->sram_reg); > > > > > > + if (!IS_ERR(drv->cci_clk)) > > > > > > + clk_put(drv->cci_clk); > > > > > > + if (!IS_ERR(drv->inter_clk)) > > > > > > + clk_put(drv->inter_clk); > > > > > > + > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > > > > +{ > > > > > > + struct device *dev = &pdev->dev; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + > > > > > > + drv = platform_get_drvdata(pdev); > > > > > > + > > > > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > > + regulator_disable(drv->proc_reg); > > > > > > + if (drv->sram_reg) > > > > > > + regulator_disable(drv->sram_reg); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static const struct mtk_ccifreq_platform_data > > > > > > mt8183_platform_data = > > > > > > { > > > > > > + .min_volt_shift = 100000, > > > > > > + .max_volt_shift = 200000, > > > > > > + .proc_max_volt = 1150000, > > > > > > + .sram_min_volt = 0, > > > > > > + .sram_max_volt = 1150000, > > > > > > +}; > > > > > > + > > > > > > +static const struct mtk_ccifreq_platform_data > > > > > > mt8186_platform_data = > > > > > > { > > > > > > + .min_volt_shift = 100000, > > > > > > + .max_volt_shift = 250000, > > > > > > + .proc_max_volt = 1118750, > > > > > > + .sram_min_volt = 850000, > > > > > > + .sram_max_volt = 1118750, > > > > > > +}; > > > > > > + > > > > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > > > > &mt8183_platform_data }, > > > > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > > > > &mt8186_platform_data }, > > > > > > + { }, > > > > > > +}; > > > > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > > > > + > > > > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > > > > + .probe = mtk_ccifreq_probe, > > > > > > + .remove = mtk_ccifreq_remove, > > > > > > + .driver = { > > > > > > + .name = "mtk-ccifreq", > > > > > > + .of_match_table = mtk_ccifreq_machines, > > > > > > + }, > > > > > > +}; > > > > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > > > > + > > > > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > > > Hi Chanwoo, > > > > > > > > > > Just a kindly ping. > > > > > Could you please give me some suggestion on this patch? > > > > > Thanks! > > > > > > > > > > BRs, > > > > > Johnson Wang > > > > > > > > > > > > > Hi Johnson, > > > > > > > > Sorry for late reply.But I replied it yesterday as following: > > > > Maybe, this reply[1] has not yet arrrived at your mail box. > > > > [1] > > > > > > > > > > > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > > > > > > > As I described on reply[1], I updated the patches on devfreq- > > > > testing > > > > branch. Could you please test your patches based on devfreq- > > > > testing > > > > branch? > > > > > > > > > > Hi Chanwoo, > > > > > > Thanks for your information. > > > I've tested this patch based on the latest devfreq-testing branch. > > > It encounters the same crash as Chen-Yu mentioned[1]. > > > > > > [1] > > > https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u > > > > Hi Johnson, > > > > Thanks for the test. I'll drop the last patch caused of crash. > > And I'll send v3 patchset right now. > > > > > > Hi Chanwoo, > > With your v3 patchset, this CCI devfreq driver works properly on the > target. > Thanks! Hi Johnson, Thanks for the test. I'll send v4 with a Tested-by tag and some typo fixup And then I'll merge them. As I know, you will send the next version of mediatek cci driver. After that, I'll merge your patches. Thanks. -- Best Regards, Chanwoo Choi
Hi Johnson, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v5.18-rc4 next-20220427] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 1cddcfdc3c683b393df1a5c9063252eb60e52818) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named 'parent_type' in 'struct devfreq_passive_data' passive_data->parent_type = CPUFREQ_PARENT_DEV; ~~~~~~~~~~~~ ^ drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared identifier 'CPUFREQ_PARENT_DEV' passive_data->parent_type = CPUFREQ_PARENT_DEV; ^ >> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format specifies type 'int' but the argument has type 'long' [-Wformat] PTR_ERR(drv->devfreq)); ^~~~~~~~~~~~~~~~~~~~~ include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err' dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap' _p_func(dev, fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ 1 warning and 2 errors generated. vim +379 drivers/devfreq/mtk-cci-devfreq.c 255 256 static int mtk_ccifreq_probe(struct platform_device *pdev) 257 { 258 struct device *dev = &pdev->dev; 259 struct mtk_ccifreq_drv *drv; 260 struct devfreq_passive_data *passive_data; 261 struct dev_pm_opp *opp; 262 unsigned long rate, opp_volt; 263 int ret; 264 265 drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); 266 if (!drv) 267 return -ENOMEM; 268 269 drv->dev = dev; 270 drv->soc_data = (const struct mtk_ccifreq_platform_data *) 271 of_device_get_match_data(&pdev->dev); 272 mutex_init(&drv->reg_lock); 273 platform_set_drvdata(pdev, drv); 274 275 drv->cci_clk = devm_clk_get(dev, "cci"); 276 if (IS_ERR(drv->cci_clk)) { 277 ret = PTR_ERR(drv->cci_clk); 278 return dev_err_probe(dev, ret, 279 "failed to get cci clk: %d\n", ret); 280 } 281 282 drv->inter_clk = devm_clk_get(dev, "intermediate"); 283 if (IS_ERR(drv->inter_clk)) { 284 ret = PTR_ERR(drv->inter_clk); 285 dev_err_probe(dev, ret, 286 "failed to get intermediate clk: %d\n", ret); 287 goto out_free_resources; 288 } 289 290 drv->proc_reg = devm_regulator_get_optional(dev, "proc"); 291 if (IS_ERR(drv->proc_reg)) { 292 ret = PTR_ERR(drv->proc_reg); 293 dev_err_probe(dev, ret, 294 "failed to get proc regulator: %d\n", ret); 295 goto out_free_resources; 296 } 297 298 ret = regulator_enable(drv->proc_reg); 299 if (ret) { 300 dev_err(dev, "failed to enable proc regulator\n"); 301 goto out_free_resources; 302 } 303 304 drv->sram_reg = regulator_get_optional(dev, "sram"); 305 if (IS_ERR(drv->sram_reg)) 306 drv->sram_reg = NULL; 307 else { 308 ret = regulator_enable(drv->sram_reg); 309 if (ret) { 310 dev_err(dev, "failed to enable sram regulator\n"); 311 goto out_free_resources; 312 } 313 } 314 315 /* 316 * We assume min voltage is 0 and tracking target voltage using 317 * min_volt_shift for each iteration. 318 * The retry_max is 3 times of expeted iteration count. 319 */ 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt, 321 drv->soc_data->proc_max_volt), 322 drv->soc_data->min_volt_shift); 323 324 ret = clk_prepare_enable(drv->cci_clk); 325 if (ret) 326 goto out_free_resources; 327 328 ret = clk_prepare_enable(drv->inter_clk); 329 if (ret) 330 goto out_disable_cci_clk; 331 332 ret = dev_pm_opp_of_add_table(dev); 333 if (ret) { 334 dev_err(dev, "failed to add opp table: %d\n", ret); 335 goto out_disable_inter_clk; 336 } 337 338 rate = clk_get_rate(drv->inter_clk); 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); 340 if (IS_ERR(opp)) { 341 ret = PTR_ERR(opp); 342 dev_err(dev, "failed to get intermediate opp: %d\n", ret); 343 goto out_remove_opp_table; 344 } 345 drv->inter_voltage = dev_pm_opp_get_voltage(opp); 346 dev_pm_opp_put(opp); 347 348 rate = U32_MAX; 349 opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); 350 if (IS_ERR(opp)) { 351 dev_err(dev, "failed to get opp\n"); 352 ret = PTR_ERR(opp); 353 goto out_remove_opp_table; 354 } 355 356 opp_volt = dev_pm_opp_get_voltage(opp); 357 dev_pm_opp_put(opp); 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); 359 if (ret) { 360 dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n", 361 opp_volt); 362 goto out_remove_opp_table; 363 } 364 365 passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data), 366 GFP_KERNEL); 367 if (!passive_data) { 368 ret = -ENOMEM; 369 goto out_remove_opp_table; 370 } 371 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; 373 drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile, 374 DEVFREQ_GOV_PASSIVE, 375 passive_data); 376 if (IS_ERR(drv->devfreq)) { 377 ret = -EPROBE_DEFER; 378 dev_err(dev, "failed to add devfreq device: %d\n", > 379 PTR_ERR(drv->devfreq)); 380 goto out_remove_opp_table; 381 } 382 383 drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; 384 ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); 385 if (ret) { 386 dev_err(dev, "failed to register opp notifier: %d\n", ret); 387 goto out_remove_devfreq_device; 388 } 389 return 0; 390 391 out_remove_devfreq_device: 392 devm_devfreq_remove_device(dev, drv->devfreq); 393 394 out_remove_opp_table: 395 dev_pm_opp_of_remove_table(dev); 396 397 out_disable_inter_clk: 398 clk_disable_unprepare(drv->inter_clk); 399 400 out_disable_cci_clk: 401 clk_disable_unprepare(drv->cci_clk); 402 403 out_free_resources: 404 if (regulator_is_enabled(drv->proc_reg)) 405 regulator_disable(drv->proc_reg); 406 if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) 407 regulator_disable(drv->sram_reg); 408 409 if (!IS_ERR(drv->proc_reg)) 410 regulator_put(drv->proc_reg); 411 if (!IS_ERR(drv->sram_reg)) 412 regulator_put(drv->sram_reg); 413 if (!IS_ERR(drv->cci_clk)) 414 clk_put(drv->cci_clk); 415 if (!IS_ERR(drv->inter_clk)) 416 clk_put(drv->inter_clk); 417 418 return ret; 419 } 420 -- 0-DAY CI Kernel Test Service https://01.org/lkp
On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > Hi Johnson, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on robh/for-next] > [also build test WARNING on linus/master v5.18-rc4 next-20220427] > [If your patch is applied to the wrong git tree, kindly drop us a > note. > And when submitting patch, we suggest to use '--base' as documented > in > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > ] > > url: > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > > base: > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > for-next > config: hexagon-allyesconfig ( > https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > ) > compiler: clang version 15.0.0 ( > https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > reproduce (this is a W=1 build): > wget > https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > > git remote add linux-review > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > > git fetch --no-tags linux-review Johnson-Wang/Introduce- > MediaTek-CCI-devfreq-driver/20220425-205820 > git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > 'parent_type' in 'struct devfreq_passive_data' > passive_data->parent_type = CPUFREQ_PARENT_DEV; > ~~~~~~~~~~~~ ^ > drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > identifier 'CPUFREQ_PARENT_DEV' > passive_data->parent_type = CPUFREQ_PARENT_DEV; > ^ > > > drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > > > specifies type 'int' but the argument has type 'long' [-Wformat] > > PTR_ERR(drv->devfreq)); > ^~~~~~~~~~~~~~~~~~~~~ > include/linux/dev_printk.h:144:65: note: expanded from macro > 'dev_err' > dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > dev_fmt(fmt), ##__VA_ARGS__) > ~~~ > ^~~~~~~~~~~ > include/linux/dev_printk.h:110:23: note: expanded from macro > 'dev_printk_index_wrap' > _p_func(dev, fmt, > ##__VA_ARGS__); \ > ~~~ ^~~~~~~~~~~ > 1 warning and 2 errors generated. > > > vim +379 drivers/devfreq/mtk-cci-devfreq.c > > 255 > 256 static int mtk_ccifreq_probe(struct platform_device > *pdev) > 257 { > 258 struct device *dev = &pdev->dev; > 259 struct mtk_ccifreq_drv *drv; > 260 struct devfreq_passive_data *passive_data; > 261 struct dev_pm_opp *opp; > 262 unsigned long rate, opp_volt; > 263 int ret; > 264 > 265 drv = devm_kzalloc(dev, sizeof(*drv), > GFP_KERNEL); > 266 if (!drv) > 267 return -ENOMEM; > 268 > 269 drv->dev = dev; > 270 drv->soc_data = (const struct > mtk_ccifreq_platform_data *) > 271 of_device_get_match_dat > a(&pdev->dev); > 272 mutex_init(&drv->reg_lock); > 273 platform_set_drvdata(pdev, drv); > 274 > 275 drv->cci_clk = devm_clk_get(dev, "cci"); > 276 if (IS_ERR(drv->cci_clk)) { > 277 ret = PTR_ERR(drv->cci_clk); > 278 return dev_err_probe(dev, ret, > 279 "failed to get cci > clk: %d\n", ret); > 280 } > 281 > 282 drv->inter_clk = devm_clk_get(dev, > "intermediate"); > 283 if (IS_ERR(drv->inter_clk)) { > 284 ret = PTR_ERR(drv->inter_clk); > 285 dev_err_probe(dev, ret, > 286 "failed to get > intermediate clk: %d\n", ret); > 287 goto out_free_resources; > 288 } > 289 > 290 drv->proc_reg = > devm_regulator_get_optional(dev, "proc"); > 291 if (IS_ERR(drv->proc_reg)) { > 292 ret = PTR_ERR(drv->proc_reg); > 293 dev_err_probe(dev, ret, > 294 "failed to get proc > regulator: %d\n", ret); > 295 goto out_free_resources; > 296 } > 297 > 298 ret = regulator_enable(drv->proc_reg); > 299 if (ret) { > 300 dev_err(dev, "failed to enable proc > regulator\n"); > 301 goto out_free_resources; > 302 } > 303 > 304 drv->sram_reg = regulator_get_optional(dev, > "sram"); > 305 if (IS_ERR(drv->sram_reg)) > 306 drv->sram_reg = NULL; > 307 else { > 308 ret = regulator_enable(drv->sram_reg); > 309 if (ret) { > 310 dev_err(dev, "failed to enable > sram regulator\n"); > 311 goto out_free_resources; > 312 } > 313 } > 314 > 315 /* > 316 * We assume min voltage is 0 and tracking > target voltage using > 317 * min_volt_shift for each iteration. > 318 * The retry_max is 3 times of expeted > iteration count. > 319 */ > 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > >soc_data->sram_max_volt, > 321 drv- > >soc_data->proc_max_volt), > 322 drv- > >soc_data->min_volt_shift); > 323 > 324 ret = clk_prepare_enable(drv->cci_clk); > 325 if (ret) > 326 goto out_free_resources; > 327 > 328 ret = clk_prepare_enable(drv->inter_clk); > 329 if (ret) > 330 goto out_disable_cci_clk; > 331 > 332 ret = dev_pm_opp_of_add_table(dev); > 333 if (ret) { > 334 dev_err(dev, "failed to add opp table: > %d\n", ret); > 335 goto out_disable_inter_clk; > 336 } > 337 > 338 rate = clk_get_rate(drv->inter_clk); > 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > 340 if (IS_ERR(opp)) { > 341 ret = PTR_ERR(opp); > 342 dev_err(dev, "failed to get > intermediate opp: %d\n", ret); > 343 goto out_remove_opp_table; > 344 } > 345 drv->inter_voltage = > dev_pm_opp_get_voltage(opp); > 346 dev_pm_opp_put(opp); > 347 > 348 rate = U32_MAX; > 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > &rate); > 350 if (IS_ERR(opp)) { > 351 dev_err(dev, "failed to get opp\n"); > 352 ret = PTR_ERR(opp); > 353 goto out_remove_opp_table; > 354 } > 355 > 356 opp_volt = dev_pm_opp_get_voltage(opp); > 357 dev_pm_opp_put(opp); > 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > 359 if (ret) { > 360 dev_err(dev, "failed to scale to > highest voltage %lu in proc_reg\n", > 361 opp_volt); > 362 goto out_remove_opp_table; > 363 } > 364 > 365 passive_data = devm_kzalloc(dev, sizeof(struct > devfreq_passive_data), > 366 GFP_KERNEL); > 367 if (!passive_data) { > 368 ret = -ENOMEM; > 369 goto out_remove_opp_table; > 370 } > 371 > 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > 373 drv->devfreq = devm_devfreq_add_device(dev, > &mtk_ccifreq_profile, > 374 DEVFREQ_ > GOV_PASSIVE, > 375 passive_ > data); > 376 if (IS_ERR(drv->devfreq)) { > 377 ret = -EPROBE_DEFER; > 378 dev_err(dev, "failed to add devfreq > device: %d\n", > > 379 PTR_ERR(drv->devfreq)); > 380 goto out_remove_opp_table; > 381 } > 382 > 383 drv->opp_nb.notifier_call = > mtk_ccifreq_opp_notifier; > 384 ret = dev_pm_opp_register_notifier(dev, &drv- > >opp_nb); > 385 if (ret) { > 386 dev_err(dev, "failed to register opp > notifier: %d\n", ret); > 387 goto out_remove_devfreq_device; > 388 } > 389 return 0; > 390 > 391 out_remove_devfreq_device: > 392 devm_devfreq_remove_device(dev, drv->devfreq); > 393 > 394 out_remove_opp_table: > 395 dev_pm_opp_of_remove_table(dev); > 396 > 397 out_disable_inter_clk: > 398 clk_disable_unprepare(drv->inter_clk); > 399 > 400 out_disable_cci_clk: > 401 clk_disable_unprepare(drv->cci_clk); > 402 > 403 out_free_resources: > 404 if (regulator_is_enabled(drv->proc_reg)) > 405 regulator_disable(drv->proc_reg); > 406 if (drv->sram_reg && regulator_is_enabled(drv- > >sram_reg)) > 407 regulator_disable(drv->sram_reg); > 408 > 409 if (!IS_ERR(drv->proc_reg)) > 410 regulator_put(drv->proc_reg); > 411 if (!IS_ERR(drv->sram_reg)) > 412 regulator_put(drv->sram_reg); > 413 if (!IS_ERR(drv->cci_clk)) > 414 clk_put(drv->cci_clk); > 415 if (!IS_ERR(drv->inter_clk)) > 416 clk_put(drv->inter_clk); > 417 > 418 return ret; > 419 } > 420 > Hi "kernel test robot", Thanks for your review. This patch is based on chanwoo/devfreq-testing[1] [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing I will follow your suggestion to use '--base' in the next version. BRs, Johnson Wang
On 4/27/2022 6:11 PM, Johnson Wang wrote: > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: >> Hi Johnson, >> >> Thank you for the patch! Perhaps something to improve: >> >> [auto build test WARNING on robh/for-next] >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] >> [If your patch is applied to the wrong git tree, kindly drop us a >> note. >> And when submitting patch, we suggest to use '--base' as documented >> in >> > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ >> ] >> >> url: >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ >> >> base: >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ >> for-next >> config: hexagon-allyesconfig ( >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ >> ) >> compiler: clang version 15.0.0 ( >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) >> reproduce (this is a W=1 build): >> wget >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ >> -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ >> >> git remote add linux-review >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ >> >> git fetch --no-tags linux-review Johnson-Wang/Introduce- >> MediaTek-CCI-devfreq-driver/20220425-205820 >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f >> # save the config file >> mkdir build_dir && cp config build_dir/.config >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ >> >> If you fix the issue, kindly add following tag as appropriate >> Reported-by: kernel test robot <lkp@intel.com> >> >> All warnings (new ones prefixed by >>): >> >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named >> 'parent_type' in 'struct devfreq_passive_data' >> passive_data->parent_type = CPUFREQ_PARENT_DEV; >> ~~~~~~~~~~~~ ^ >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared >> identifier 'CPUFREQ_PARENT_DEV' >> passive_data->parent_type = CPUFREQ_PARENT_DEV; >> ^ >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format >>>> specifies type 'int' but the argument has type 'long' [-Wformat] >> >> PTR_ERR(drv->devfreq)); >> ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/dev_printk.h:144:65: note: expanded from macro >> 'dev_err' >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, >> dev_fmt(fmt), ##__VA_ARGS__) >> ~~~ >> ^~~~~~~~~~~ >> include/linux/dev_printk.h:110:23: note: expanded from macro >> 'dev_printk_index_wrap' >> _p_func(dev, fmt, >> ##__VA_ARGS__); \ >> ~~~ ^~~~~~~~~~~ >> 1 warning and 2 errors generated. >> >> >> vim +379 drivers/devfreq/mtk-cci-devfreq.c >> >> 255 >> 256 static int mtk_ccifreq_probe(struct platform_device >> *pdev) >> 257 { >> 258 struct device *dev = &pdev->dev; >> 259 struct mtk_ccifreq_drv *drv; >> 260 struct devfreq_passive_data *passive_data; >> 261 struct dev_pm_opp *opp; >> 262 unsigned long rate, opp_volt; >> 263 int ret; >> 264 >> 265 drv = devm_kzalloc(dev, sizeof(*drv), >> GFP_KERNEL); >> 266 if (!drv) >> 267 return -ENOMEM; >> 268 >> 269 drv->dev = dev; >> 270 drv->soc_data = (const struct >> mtk_ccifreq_platform_data *) >> 271 of_device_get_match_dat >> a(&pdev->dev); >> 272 mutex_init(&drv->reg_lock); >> 273 platform_set_drvdata(pdev, drv); >> 274 >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); >> 276 if (IS_ERR(drv->cci_clk)) { >> 277 ret = PTR_ERR(drv->cci_clk); >> 278 return dev_err_probe(dev, ret, >> 279 "failed to get cci >> clk: %d\n", ret); >> 280 } >> 281 >> 282 drv->inter_clk = devm_clk_get(dev, >> "intermediate"); >> 283 if (IS_ERR(drv->inter_clk)) { >> 284 ret = PTR_ERR(drv->inter_clk); >> 285 dev_err_probe(dev, ret, >> 286 "failed to get >> intermediate clk: %d\n", ret); >> 287 goto out_free_resources; >> 288 } >> 289 >> 290 drv->proc_reg = >> devm_regulator_get_optional(dev, "proc"); >> 291 if (IS_ERR(drv->proc_reg)) { >> 292 ret = PTR_ERR(drv->proc_reg); >> 293 dev_err_probe(dev, ret, >> 294 "failed to get proc >> regulator: %d\n", ret); >> 295 goto out_free_resources; >> 296 } >> 297 >> 298 ret = regulator_enable(drv->proc_reg); >> 299 if (ret) { >> 300 dev_err(dev, "failed to enable proc >> regulator\n"); >> 301 goto out_free_resources; >> 302 } >> 303 >> 304 drv->sram_reg = regulator_get_optional(dev, >> "sram"); >> 305 if (IS_ERR(drv->sram_reg)) >> 306 drv->sram_reg = NULL; >> 307 else { >> 308 ret = regulator_enable(drv->sram_reg); >> 309 if (ret) { >> 310 dev_err(dev, "failed to enable >> sram regulator\n"); >> 311 goto out_free_resources; >> 312 } >> 313 } >> 314 >> 315 /* >> 316 * We assume min voltage is 0 and tracking >> target voltage using >> 317 * min_volt_shift for each iteration. >> 318 * The retry_max is 3 times of expeted >> iteration count. >> 319 */ >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- >>> soc_data->sram_max_volt, >> 321 drv- >>> soc_data->proc_max_volt), >> 322 drv- >>> soc_data->min_volt_shift); >> 323 >> 324 ret = clk_prepare_enable(drv->cci_clk); >> 325 if (ret) >> 326 goto out_free_resources; >> 327 >> 328 ret = clk_prepare_enable(drv->inter_clk); >> 329 if (ret) >> 330 goto out_disable_cci_clk; >> 331 >> 332 ret = dev_pm_opp_of_add_table(dev); >> 333 if (ret) { >> 334 dev_err(dev, "failed to add opp table: >> %d\n", ret); >> 335 goto out_disable_inter_clk; >> 336 } >> 337 >> 338 rate = clk_get_rate(drv->inter_clk); >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); >> 340 if (IS_ERR(opp)) { >> 341 ret = PTR_ERR(opp); >> 342 dev_err(dev, "failed to get >> intermediate opp: %d\n", ret); >> 343 goto out_remove_opp_table; >> 344 } >> 345 drv->inter_voltage = >> dev_pm_opp_get_voltage(opp); >> 346 dev_pm_opp_put(opp); >> 347 >> 348 rate = U32_MAX; >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, >> &rate); >> 350 if (IS_ERR(opp)) { >> 351 dev_err(dev, "failed to get opp\n"); >> 352 ret = PTR_ERR(opp); >> 353 goto out_remove_opp_table; >> 354 } >> 355 >> 356 opp_volt = dev_pm_opp_get_voltage(opp); >> 357 dev_pm_opp_put(opp); >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); >> 359 if (ret) { >> 360 dev_err(dev, "failed to scale to >> highest voltage %lu in proc_reg\n", >> 361 opp_volt); >> 362 goto out_remove_opp_table; >> 363 } >> 364 >> 365 passive_data = devm_kzalloc(dev, sizeof(struct >> devfreq_passive_data), >> 366 GFP_KERNEL); >> 367 if (!passive_data) { >> 368 ret = -ENOMEM; >> 369 goto out_remove_opp_table; >> 370 } >> 371 >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; >> 373 drv->devfreq = devm_devfreq_add_device(dev, >> &mtk_ccifreq_profile, >> 374 DEVFREQ_ >> GOV_PASSIVE, >> 375 passive_ >> data); >> 376 if (IS_ERR(drv->devfreq)) { >> 377 ret = -EPROBE_DEFER; >> 378 dev_err(dev, "failed to add devfreq >> device: %d\n", >> > 379 PTR_ERR(drv->devfreq)); >> 380 goto out_remove_opp_table; >> 381 } >> 382 >> 383 drv->opp_nb.notifier_call = >> mtk_ccifreq_opp_notifier; >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- >>> opp_nb); >> 385 if (ret) { >> 386 dev_err(dev, "failed to register opp >> notifier: %d\n", ret); >> 387 goto out_remove_devfreq_device; >> 388 } >> 389 return 0; >> 390 >> 391 out_remove_devfreq_device: >> 392 devm_devfreq_remove_device(dev, drv->devfreq); >> 393 >> 394 out_remove_opp_table: >> 395 dev_pm_opp_of_remove_table(dev); >> 396 >> 397 out_disable_inter_clk: >> 398 clk_disable_unprepare(drv->inter_clk); >> 399 >> 400 out_disable_cci_clk: >> 401 clk_disable_unprepare(drv->cci_clk); >> 402 >> 403 out_free_resources: >> 404 if (regulator_is_enabled(drv->proc_reg)) >> 405 regulator_disable(drv->proc_reg); >> 406 if (drv->sram_reg && regulator_is_enabled(drv- >>> sram_reg)) >> 407 regulator_disable(drv->sram_reg); >> 408 >> 409 if (!IS_ERR(drv->proc_reg)) >> 410 regulator_put(drv->proc_reg); >> 411 if (!IS_ERR(drv->sram_reg)) >> 412 regulator_put(drv->sram_reg); >> 413 if (!IS_ERR(drv->cci_clk)) >> 414 clk_put(drv->cci_clk); >> 415 if (!IS_ERR(drv->inter_clk)) >> 416 clk_put(drv->inter_clk); >> 417 >> 418 return ret; >> 419 } >> 420 >> > > Hi "kernel test robot", > > Thanks for your review. > > This patch is based on chanwoo/devfreq-testing[1] > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing Hi Johnson, Thanks for the feedback, we'll take a look too. Best Regards, Rong Chen > > I will follow your suggestion to use '--base' in the next version. > > BRs, > Johnson Wang > _______________________________________________ > kbuild-all mailing list -- kbuild-all@lists.01.org > To unsubscribe send an email to kbuild-all-leave@lists.01.org >
On Thu, Apr 28, 2022 at 7:39 PM Chen, Rong A <rong.a.chen@intel.com> wrote: > On 4/27/2022 6:11 PM, Johnson Wang wrote: > > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > >> Hi Johnson, > >> > >> Thank you for the patch! Perhaps something to improve: > >> > >> [auto build test WARNING on robh/for-next] > >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] > >> [If your patch is applied to the wrong git tree, kindly drop us a > >> note. > >> And when submitting patch, we suggest to use '--base' as documented > >> in > >> > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > >> ] > >> > >> url: > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > >> > >> base: > >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > >> for-next > >> config: hexagon-allyesconfig ( > >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > >> ) > >> compiler: clang version 15.0.0 ( > >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > >> reproduce (this is a W=1 build): > >> wget > >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > >> -O ~/bin/make.cross > >> chmod +x ~/bin/make.cross > >> # > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > >> > >> git remote add linux-review > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > >> > >> git fetch --no-tags linux-review Johnson-Wang/Introduce- > >> MediaTek-CCI-devfreq-driver/20220425-205820 > >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > >> # save the config file > >> mkdir build_dir && cp config build_dir/.config > >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > >> > >> If you fix the issue, kindly add following tag as appropriate > >> Reported-by: kernel test robot <lkp@intel.com> > >> > >> All warnings (new ones prefixed by >>): > >> > >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > >> 'parent_type' in 'struct devfreq_passive_data' > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> ~~~~~~~~~~~~ ^ > >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > >> identifier 'CPUFREQ_PARENT_DEV' > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> ^ > >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > >>>> specifies type 'int' but the argument has type 'long' [-Wformat] > >> > >> PTR_ERR(drv->devfreq)); > >> ^~~~~~~~~~~~~~~~~~~~~ > >> include/linux/dev_printk.h:144:65: note: expanded from macro > >> 'dev_err' > >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > >> dev_fmt(fmt), ##__VA_ARGS__) > >> ~~~ > >> ^~~~~~~~~~~ > >> include/linux/dev_printk.h:110:23: note: expanded from macro > >> 'dev_printk_index_wrap' > >> _p_func(dev, fmt, > >> ##__VA_ARGS__); \ > >> ~~~ ^~~~~~~~~~~ > >> 1 warning and 2 errors generated. > >> > >> > >> vim +379 drivers/devfreq/mtk-cci-devfreq.c > >> > >> 255 > >> 256 static int mtk_ccifreq_probe(struct platform_device > >> *pdev) > >> 257 { > >> 258 struct device *dev = &pdev->dev; > >> 259 struct mtk_ccifreq_drv *drv; > >> 260 struct devfreq_passive_data *passive_data; > >> 261 struct dev_pm_opp *opp; > >> 262 unsigned long rate, opp_volt; > >> 263 int ret; > >> 264 > >> 265 drv = devm_kzalloc(dev, sizeof(*drv), > >> GFP_KERNEL); > >> 266 if (!drv) > >> 267 return -ENOMEM; > >> 268 > >> 269 drv->dev = dev; > >> 270 drv->soc_data = (const struct > >> mtk_ccifreq_platform_data *) > >> 271 of_device_get_match_dat > >> a(&pdev->dev); > >> 272 mutex_init(&drv->reg_lock); > >> 273 platform_set_drvdata(pdev, drv); > >> 274 > >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); > >> 276 if (IS_ERR(drv->cci_clk)) { > >> 277 ret = PTR_ERR(drv->cci_clk); > >> 278 return dev_err_probe(dev, ret, > >> 279 "failed to get cci > >> clk: %d\n", ret); > >> 280 } > >> 281 > >> 282 drv->inter_clk = devm_clk_get(dev, > >> "intermediate"); > >> 283 if (IS_ERR(drv->inter_clk)) { > >> 284 ret = PTR_ERR(drv->inter_clk); > >> 285 dev_err_probe(dev, ret, > >> 286 "failed to get > >> intermediate clk: %d\n", ret); > >> 287 goto out_free_resources; > >> 288 } > >> 289 > >> 290 drv->proc_reg = > >> devm_regulator_get_optional(dev, "proc"); > >> 291 if (IS_ERR(drv->proc_reg)) { > >> 292 ret = PTR_ERR(drv->proc_reg); > >> 293 dev_err_probe(dev, ret, > >> 294 "failed to get proc > >> regulator: %d\n", ret); > >> 295 goto out_free_resources; > >> 296 } > >> 297 > >> 298 ret = regulator_enable(drv->proc_reg); > >> 299 if (ret) { > >> 300 dev_err(dev, "failed to enable proc > >> regulator\n"); > >> 301 goto out_free_resources; > >> 302 } > >> 303 > >> 304 drv->sram_reg = regulator_get_optional(dev, > >> "sram"); > >> 305 if (IS_ERR(drv->sram_reg)) > >> 306 drv->sram_reg = NULL; > >> 307 else { > >> 308 ret = regulator_enable(drv->sram_reg); > >> 309 if (ret) { > >> 310 dev_err(dev, "failed to enable > >> sram regulator\n"); > >> 311 goto out_free_resources; > >> 312 } > >> 313 } > >> 314 > >> 315 /* > >> 316 * We assume min voltage is 0 and tracking > >> target voltage using > >> 317 * min_volt_shift for each iteration. > >> 318 * The retry_max is 3 times of expeted > >> iteration count. > >> 319 */ > >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > >>> soc_data->sram_max_volt, > >> 321 drv- > >>> soc_data->proc_max_volt), > >> 322 drv- > >>> soc_data->min_volt_shift); > >> 323 > >> 324 ret = clk_prepare_enable(drv->cci_clk); > >> 325 if (ret) > >> 326 goto out_free_resources; > >> 327 > >> 328 ret = clk_prepare_enable(drv->inter_clk); > >> 329 if (ret) > >> 330 goto out_disable_cci_clk; > >> 331 > >> 332 ret = dev_pm_opp_of_add_table(dev); > >> 333 if (ret) { > >> 334 dev_err(dev, "failed to add opp table: > >> %d\n", ret); > >> 335 goto out_disable_inter_clk; > >> 336 } > >> 337 > >> 338 rate = clk_get_rate(drv->inter_clk); > >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > >> 340 if (IS_ERR(opp)) { > >> 341 ret = PTR_ERR(opp); > >> 342 dev_err(dev, "failed to get > >> intermediate opp: %d\n", ret); > >> 343 goto out_remove_opp_table; > >> 344 } > >> 345 drv->inter_voltage = > >> dev_pm_opp_get_voltage(opp); > >> 346 dev_pm_opp_put(opp); > >> 347 > >> 348 rate = U32_MAX; > >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > >> &rate); > >> 350 if (IS_ERR(opp)) { > >> 351 dev_err(dev, "failed to get opp\n"); > >> 352 ret = PTR_ERR(opp); > >> 353 goto out_remove_opp_table; > >> 354 } > >> 355 > >> 356 opp_volt = dev_pm_opp_get_voltage(opp); > >> 357 dev_pm_opp_put(opp); > >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > >> 359 if (ret) { > >> 360 dev_err(dev, "failed to scale to > >> highest voltage %lu in proc_reg\n", > >> 361 opp_volt); > >> 362 goto out_remove_opp_table; > >> 363 } > >> 364 > >> 365 passive_data = devm_kzalloc(dev, sizeof(struct > >> devfreq_passive_data), > >> 366 GFP_KERNEL); > >> 367 if (!passive_data) { > >> 368 ret = -ENOMEM; > >> 369 goto out_remove_opp_table; > >> 370 } > >> 371 > >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> 373 drv->devfreq = devm_devfreq_add_device(dev, > >> &mtk_ccifreq_profile, > >> 374 DEVFREQ_ > >> GOV_PASSIVE, > >> 375 passive_ > >> data); > >> 376 if (IS_ERR(drv->devfreq)) { > >> 377 ret = -EPROBE_DEFER; > >> 378 dev_err(dev, "failed to add devfreq > >> device: %d\n", > >> > 379 PTR_ERR(drv->devfreq)); > >> 380 goto out_remove_opp_table; > >> 381 } > >> 382 > >> 383 drv->opp_nb.notifier_call = > >> mtk_ccifreq_opp_notifier; > >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- > >>> opp_nb); > >> 385 if (ret) { > >> 386 dev_err(dev, "failed to register opp > >> notifier: %d\n", ret); > >> 387 goto out_remove_devfreq_device; > >> 388 } > >> 389 return 0; > >> 390 > >> 391 out_remove_devfreq_device: > >> 392 devm_devfreq_remove_device(dev, drv->devfreq); > >> 393 > >> 394 out_remove_opp_table: > >> 395 dev_pm_opp_of_remove_table(dev); > >> 396 > >> 397 out_disable_inter_clk: > >> 398 clk_disable_unprepare(drv->inter_clk); > >> 399 > >> 400 out_disable_cci_clk: > >> 401 clk_disable_unprepare(drv->cci_clk); > >> 402 > >> 403 out_free_resources: > >> 404 if (regulator_is_enabled(drv->proc_reg)) > >> 405 regulator_disable(drv->proc_reg); > >> 406 if (drv->sram_reg && regulator_is_enabled(drv- > >>> sram_reg)) > >> 407 regulator_disable(drv->sram_reg); > >> 408 > >> 409 if (!IS_ERR(drv->proc_reg)) > >> 410 regulator_put(drv->proc_reg); > >> 411 if (!IS_ERR(drv->sram_reg)) > >> 412 regulator_put(drv->sram_reg); > >> 413 if (!IS_ERR(drv->cci_clk)) > >> 414 clk_put(drv->cci_clk); > >> 415 if (!IS_ERR(drv->inter_clk)) > >> 416 clk_put(drv->inter_clk); > >> 417 > >> 418 return ret; > >> 419 } > >> 420 > >> > > > > Hi "kernel test robot", > > > > Thanks for your review. > > > > This patch is based on chanwoo/devfreq-testing[1] > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > > Hi Johnson, > > Thanks for the feedback, we'll take a look too. I think the last patch on that branch might be broken. ChenYu
Hi Chen-Yu, On Mon, May 9, 2022 at 10:53 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > On Thu, Apr 28, 2022 at 7:39 PM Chen, Rong A <rong.a.chen@intel.com> wrote: > > On 4/27/2022 6:11 PM, Johnson Wang wrote: > > > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > > >> Hi Johnson, > > >> > > >> Thank you for the patch! Perhaps something to improve: > > >> > > >> [auto build test WARNING on robh/for-next] > > >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] > > >> [If your patch is applied to the wrong git tree, kindly drop us a > > >> note. > > >> And when submitting patch, we suggest to use '--base' as documented > > >> in > > >> > > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > > >> ] > > >> > > >> url: > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > > >> > > >> base: > > >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > > >> for-next > > >> config: hexagon-allyesconfig ( > > >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > > >> ) > > >> compiler: clang version 15.0.0 ( > > >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > > >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > > >> reproduce (this is a W=1 build): > > >> wget > > >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > > >> -O ~/bin/make.cross > > >> chmod +x ~/bin/make.cross > > >> # > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > > >> > > >> git remote add linux-review > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > > >> > > >> git fetch --no-tags linux-review Johnson-Wang/Introduce- > > >> MediaTek-CCI-devfreq-driver/20220425-205820 > > >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > > >> # save the config file > > >> mkdir build_dir && cp config build_dir/.config > > >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > > >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > > >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > > >> > > >> If you fix the issue, kindly add following tag as appropriate > > >> Reported-by: kernel test robot <lkp@intel.com> > > >> > > >> All warnings (new ones prefixed by >>): > > >> > > >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > > >> 'parent_type' in 'struct devfreq_passive_data' > > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> ~~~~~~~~~~~~ ^ > > >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > > >> identifier 'CPUFREQ_PARENT_DEV' > > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> ^ > > >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > > >>>> specifies type 'int' but the argument has type 'long' [-Wformat] > > >> > > >> PTR_ERR(drv->devfreq)); > > >> ^~~~~~~~~~~~~~~~~~~~~ > > >> include/linux/dev_printk.h:144:65: note: expanded from macro > > >> 'dev_err' > > >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > > >> dev_fmt(fmt), ##__VA_ARGS__) > > >> ~~~ > > >> ^~~~~~~~~~~ > > >> include/linux/dev_printk.h:110:23: note: expanded from macro > > >> 'dev_printk_index_wrap' > > >> _p_func(dev, fmt, > > >> ##__VA_ARGS__); \ > > >> ~~~ ^~~~~~~~~~~ > > >> 1 warning and 2 errors generated. > > >> > > >> > > >> vim +379 drivers/devfreq/mtk-cci-devfreq.c > > >> > > >> 255 > > >> 256 static int mtk_ccifreq_probe(struct platform_device > > >> *pdev) > > >> 257 { > > >> 258 struct device *dev = &pdev->dev; > > >> 259 struct mtk_ccifreq_drv *drv; > > >> 260 struct devfreq_passive_data *passive_data; > > >> 261 struct dev_pm_opp *opp; > > >> 262 unsigned long rate, opp_volt; > > >> 263 int ret; > > >> 264 > > >> 265 drv = devm_kzalloc(dev, sizeof(*drv), > > >> GFP_KERNEL); > > >> 266 if (!drv) > > >> 267 return -ENOMEM; > > >> 268 > > >> 269 drv->dev = dev; > > >> 270 drv->soc_data = (const struct > > >> mtk_ccifreq_platform_data *) > > >> 271 of_device_get_match_dat > > >> a(&pdev->dev); > > >> 272 mutex_init(&drv->reg_lock); > > >> 273 platform_set_drvdata(pdev, drv); > > >> 274 > > >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); > > >> 276 if (IS_ERR(drv->cci_clk)) { > > >> 277 ret = PTR_ERR(drv->cci_clk); > > >> 278 return dev_err_probe(dev, ret, > > >> 279 "failed to get cci > > >> clk: %d\n", ret); > > >> 280 } > > >> 281 > > >> 282 drv->inter_clk = devm_clk_get(dev, > > >> "intermediate"); > > >> 283 if (IS_ERR(drv->inter_clk)) { > > >> 284 ret = PTR_ERR(drv->inter_clk); > > >> 285 dev_err_probe(dev, ret, > > >> 286 "failed to get > > >> intermediate clk: %d\n", ret); > > >> 287 goto out_free_resources; > > >> 288 } > > >> 289 > > >> 290 drv->proc_reg = > > >> devm_regulator_get_optional(dev, "proc"); > > >> 291 if (IS_ERR(drv->proc_reg)) { > > >> 292 ret = PTR_ERR(drv->proc_reg); > > >> 293 dev_err_probe(dev, ret, > > >> 294 "failed to get proc > > >> regulator: %d\n", ret); > > >> 295 goto out_free_resources; > > >> 296 } > > >> 297 > > >> 298 ret = regulator_enable(drv->proc_reg); > > >> 299 if (ret) { > > >> 300 dev_err(dev, "failed to enable proc > > >> regulator\n"); > > >> 301 goto out_free_resources; > > >> 302 } > > >> 303 > > >> 304 drv->sram_reg = regulator_get_optional(dev, > > >> "sram"); > > >> 305 if (IS_ERR(drv->sram_reg)) > > >> 306 drv->sram_reg = NULL; > > >> 307 else { > > >> 308 ret = regulator_enable(drv->sram_reg); > > >> 309 if (ret) { > > >> 310 dev_err(dev, "failed to enable > > >> sram regulator\n"); > > >> 311 goto out_free_resources; > > >> 312 } > > >> 313 } > > >> 314 > > >> 315 /* > > >> 316 * We assume min voltage is 0 and tracking > > >> target voltage using > > >> 317 * min_volt_shift for each iteration. > > >> 318 * The retry_max is 3 times of expeted > > >> iteration count. > > >> 319 */ > > >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > > >>> soc_data->sram_max_volt, > > >> 321 drv- > > >>> soc_data->proc_max_volt), > > >> 322 drv- > > >>> soc_data->min_volt_shift); > > >> 323 > > >> 324 ret = clk_prepare_enable(drv->cci_clk); > > >> 325 if (ret) > > >> 326 goto out_free_resources; > > >> 327 > > >> 328 ret = clk_prepare_enable(drv->inter_clk); > > >> 329 if (ret) > > >> 330 goto out_disable_cci_clk; > > >> 331 > > >> 332 ret = dev_pm_opp_of_add_table(dev); > > >> 333 if (ret) { > > >> 334 dev_err(dev, "failed to add opp table: > > >> %d\n", ret); > > >> 335 goto out_disable_inter_clk; > > >> 336 } > > >> 337 > > >> 338 rate = clk_get_rate(drv->inter_clk); > > >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > >> 340 if (IS_ERR(opp)) { > > >> 341 ret = PTR_ERR(opp); > > >> 342 dev_err(dev, "failed to get > > >> intermediate opp: %d\n", ret); > > >> 343 goto out_remove_opp_table; > > >> 344 } > > >> 345 drv->inter_voltage = > > >> dev_pm_opp_get_voltage(opp); > > >> 346 dev_pm_opp_put(opp); > > >> 347 > > >> 348 rate = U32_MAX; > > >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > > >> &rate); > > >> 350 if (IS_ERR(opp)) { > > >> 351 dev_err(dev, "failed to get opp\n"); > > >> 352 ret = PTR_ERR(opp); > > >> 353 goto out_remove_opp_table; > > >> 354 } > > >> 355 > > >> 356 opp_volt = dev_pm_opp_get_voltage(opp); > > >> 357 dev_pm_opp_put(opp); > > >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > >> 359 if (ret) { > > >> 360 dev_err(dev, "failed to scale to > > >> highest voltage %lu in proc_reg\n", > > >> 361 opp_volt); > > >> 362 goto out_remove_opp_table; > > >> 363 } > > >> 364 > > >> 365 passive_data = devm_kzalloc(dev, sizeof(struct > > >> devfreq_passive_data), > > >> 366 GFP_KERNEL); > > >> 367 if (!passive_data) { > > >> 368 ret = -ENOMEM; > > >> 369 goto out_remove_opp_table; > > >> 370 } > > >> 371 > > >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> 373 drv->devfreq = devm_devfreq_add_device(dev, > > >> &mtk_ccifreq_profile, > > >> 374 DEVFREQ_ > > >> GOV_PASSIVE, > > >> 375 passive_ > > >> data); > > >> 376 if (IS_ERR(drv->devfreq)) { > > >> 377 ret = -EPROBE_DEFER; > > >> 378 dev_err(dev, "failed to add devfreq > > >> device: %d\n", > > >> > 379 PTR_ERR(drv->devfreq)); > > >> 380 goto out_remove_opp_table; > > >> 381 } > > >> 382 > > >> 383 drv->opp_nb.notifier_call = > > >> mtk_ccifreq_opp_notifier; > > >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- > > >>> opp_nb); > > >> 385 if (ret) { > > >> 386 dev_err(dev, "failed to register opp > > >> notifier: %d\n", ret); > > >> 387 goto out_remove_devfreq_device; > > >> 388 } > > >> 389 return 0; > > >> 390 > > >> 391 out_remove_devfreq_device: > > >> 392 devm_devfreq_remove_device(dev, drv->devfreq); > > >> 393 > > >> 394 out_remove_opp_table: > > >> 395 dev_pm_opp_of_remove_table(dev); > > >> 396 > > >> 397 out_disable_inter_clk: > > >> 398 clk_disable_unprepare(drv->inter_clk); > > >> 399 > > >> 400 out_disable_cci_clk: > > >> 401 clk_disable_unprepare(drv->cci_clk); > > >> 402 > > >> 403 out_free_resources: > > >> 404 if (regulator_is_enabled(drv->proc_reg)) > > >> 405 regulator_disable(drv->proc_reg); > > >> 406 if (drv->sram_reg && regulator_is_enabled(drv- > > >>> sram_reg)) > > >> 407 regulator_disable(drv->sram_reg); > > >> 408 > > >> 409 if (!IS_ERR(drv->proc_reg)) > > >> 410 regulator_put(drv->proc_reg); > > >> 411 if (!IS_ERR(drv->sram_reg)) > > >> 412 regulator_put(drv->sram_reg); > > >> 413 if (!IS_ERR(drv->cci_clk)) > > >> 414 clk_put(drv->cci_clk); > > >> 415 if (!IS_ERR(drv->inter_clk)) > > >> 416 clk_put(drv->inter_clk); > > >> 417 > > >> 418 return ret; > > >> 419 } > > >> 420 > > >> > > > > > > Hi "kernel test robot", > > > > > > Thanks for your review. > > > > > > This patch is based on chanwoo/devfreq-testing[1] > > > [1] > > > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > > > > Hi Johnson, > > > > Thanks for the feedback, we'll take a look too. > > I think the last patch on that branch might be broken. You mean the patch[1]. Without this patch[1], there are no problems? [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=ea1011fba665b95fc28f682c9b131799a88b11ae When you tested these patches with patchset[2] without last patch[1] if there are no problems, please reply to patchset[2] with your Tested-by tag. [2] https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/ -- Best Regards, Chanwoo Choi
Hi Johnson, Thank you for the patch! Yet something to improve: [auto build test ERROR on robh/for-next] [also build test ERROR on linus/master v5.18-rc4 next-20220426] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271534.xh4s4n5E-lkp@intel.com/config) compiler: arceb-elf-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/devfreq/mtk-cci-devfreq.c: In function 'mtk_ccifreq_probe': >> drivers/devfreq/mtk-cci-devfreq.c:372:21: error: 'struct devfreq_passive_data' has no member named 'parent_type' 372 | passive_data->parent_type = CPUFREQ_PARENT_DEV; | ^~ >> drivers/devfreq/mtk-cci-devfreq.c:372:37: error: 'CPUFREQ_PARENT_DEV' undeclared (first use in this function) 372 | passive_data->parent_type = CPUFREQ_PARENT_DEV; | ^~~~~~~~~~~~~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:372:37: note: each undeclared identifier is reported only once for each function it appears in In file included from include/linux/device.h:15, from include/linux/devfreq.h:13, from drivers/devfreq/mtk-cci-devfreq.c:7: drivers/devfreq/mtk-cci-devfreq.c:378:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=] 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt' 144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:378:17: note: in expansion of macro 'dev_err' 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ^~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:378:62: note: format string is defined here 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ~^ | | | int | %ld vim +372 drivers/devfreq/mtk-cci-devfreq.c 255 256 static int mtk_ccifreq_probe(struct platform_device *pdev) 257 { 258 struct device *dev = &pdev->dev; 259 struct mtk_ccifreq_drv *drv; 260 struct devfreq_passive_data *passive_data; 261 struct dev_pm_opp *opp; 262 unsigned long rate, opp_volt; 263 int ret; 264 265 drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); 266 if (!drv) 267 return -ENOMEM; 268 269 drv->dev = dev; 270 drv->soc_data = (const struct mtk_ccifreq_platform_data *) 271 of_device_get_match_data(&pdev->dev); 272 mutex_init(&drv->reg_lock); 273 platform_set_drvdata(pdev, drv); 274 275 drv->cci_clk = devm_clk_get(dev, "cci"); 276 if (IS_ERR(drv->cci_clk)) { 277 ret = PTR_ERR(drv->cci_clk); 278 return dev_err_probe(dev, ret, 279 "failed to get cci clk: %d\n", ret); 280 } 281 282 drv->inter_clk = devm_clk_get(dev, "intermediate"); 283 if (IS_ERR(drv->inter_clk)) { 284 ret = PTR_ERR(drv->inter_clk); 285 dev_err_probe(dev, ret, 286 "failed to get intermediate clk: %d\n", ret); 287 goto out_free_resources; 288 } 289 290 drv->proc_reg = devm_regulator_get_optional(dev, "proc"); 291 if (IS_ERR(drv->proc_reg)) { 292 ret = PTR_ERR(drv->proc_reg); 293 dev_err_probe(dev, ret, 294 "failed to get proc regulator: %d\n", ret); 295 goto out_free_resources; 296 } 297 298 ret = regulator_enable(drv->proc_reg); 299 if (ret) { 300 dev_err(dev, "failed to enable proc regulator\n"); 301 goto out_free_resources; 302 } 303 304 drv->sram_reg = regulator_get_optional(dev, "sram"); 305 if (IS_ERR(drv->sram_reg)) 306 drv->sram_reg = NULL; 307 else { 308 ret = regulator_enable(drv->sram_reg); 309 if (ret) { 310 dev_err(dev, "failed to enable sram regulator\n"); 311 goto out_free_resources; 312 } 313 } 314 315 /* 316 * We assume min voltage is 0 and tracking target voltage using 317 * min_volt_shift for each iteration. 318 * The retry_max is 3 times of expeted iteration count. 319 */ 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt, 321 drv->soc_data->proc_max_volt), 322 drv->soc_data->min_volt_shift); 323 324 ret = clk_prepare_enable(drv->cci_clk); 325 if (ret) 326 goto out_free_resources; 327 328 ret = clk_prepare_enable(drv->inter_clk); 329 if (ret) 330 goto out_disable_cci_clk; 331 332 ret = dev_pm_opp_of_add_table(dev); 333 if (ret) { 334 dev_err(dev, "failed to add opp table: %d\n", ret); 335 goto out_disable_inter_clk; 336 } 337 338 rate = clk_get_rate(drv->inter_clk); 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); 340 if (IS_ERR(opp)) { 341 ret = PTR_ERR(opp); 342 dev_err(dev, "failed to get intermediate opp: %d\n", ret); 343 goto out_remove_opp_table; 344 } 345 drv->inter_voltage = dev_pm_opp_get_voltage(opp); 346 dev_pm_opp_put(opp); 347 348 rate = U32_MAX; 349 opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); 350 if (IS_ERR(opp)) { 351 dev_err(dev, "failed to get opp\n"); 352 ret = PTR_ERR(opp); 353 goto out_remove_opp_table; 354 } 355 356 opp_volt = dev_pm_opp_get_voltage(opp); 357 dev_pm_opp_put(opp); 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); 359 if (ret) { 360 dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n", 361 opp_volt); 362 goto out_remove_opp_table; 363 } 364 365 passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data), 366 GFP_KERNEL); 367 if (!passive_data) { 368 ret = -ENOMEM; 369 goto out_remove_opp_table; 370 } 371 > 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; 373 drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile, 374 DEVFREQ_GOV_PASSIVE, 375 passive_data); 376 if (IS_ERR(drv->devfreq)) { 377 ret = -EPROBE_DEFER; 378 dev_err(dev, "failed to add devfreq device: %d\n", 379 PTR_ERR(drv->devfreq)); 380 goto out_remove_opp_table; 381 } 382 383 drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; 384 ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); 385 if (ret) { 386 dev_err(dev, "failed to register opp notifier: %d\n", ret); 387 goto out_remove_devfreq_device; 388 } 389 return 0; 390 391 out_remove_devfreq_device: 392 devm_devfreq_remove_device(dev, drv->devfreq); 393 394 out_remove_opp_table: 395 dev_pm_opp_of_remove_table(dev); 396 397 out_disable_inter_clk: 398 clk_disable_unprepare(drv->inter_clk); 399 400 out_disable_cci_clk: 401 clk_disable_unprepare(drv->cci_clk); 402 403 out_free_resources: 404 if (regulator_is_enabled(drv->proc_reg)) 405 regulator_disable(drv->proc_reg); 406 if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) 407 regulator_disable(drv->sram_reg); 408 409 if (!IS_ERR(drv->proc_reg)) 410 regulator_put(drv->proc_reg); 411 if (!IS_ERR(drv->sram_reg)) 412 regulator_put(drv->sram_reg); 413 if (!IS_ERR(drv->cci_clk)) 414 clk_put(drv->cci_clk); 415 if (!IS_ERR(drv->inter_clk)) 416 clk_put(drv->inter_clk); 417 418 return ret; 419 } 420 -- 0-DAY CI Kernel Test Service https://01.org/lkp
Hi Johnson, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v5.18-rc4 next-20220422] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220426/202204262037.g8ZK9iOF-lkp@intel.com/config) compiler: arceb-elf-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820 git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/devfreq/ sound/core/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/devfreq/mtk-cci-devfreq.c: In function 'mtk_ccifreq_probe': drivers/devfreq/mtk-cci-devfreq.c:372:21: error: 'struct devfreq_passive_data' has no member named 'parent_type' 372 | passive_data->parent_type = CPUFREQ_PARENT_DEV; | ^~ drivers/devfreq/mtk-cci-devfreq.c:372:37: error: 'CPUFREQ_PARENT_DEV' undeclared (first use in this function) 372 | passive_data->parent_type = CPUFREQ_PARENT_DEV; | ^~~~~~~~~~~~~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:372:37: note: each undeclared identifier is reported only once for each function it appears in In file included from include/linux/device.h:15, from include/linux/devfreq.h:13, from drivers/devfreq/mtk-cci-devfreq.c:7: >> drivers/devfreq/mtk-cci-devfreq.c:378:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=] 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt' 144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:378:17: note: in expansion of macro 'dev_err' 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ^~~~~~~ drivers/devfreq/mtk-cci-devfreq.c:378:62: note: format string is defined here 378 | dev_err(dev, "failed to add devfreq device: %d\n", | ~^ | | | int | %ld vim +378 drivers/devfreq/mtk-cci-devfreq.c 255 256 static int mtk_ccifreq_probe(struct platform_device *pdev) 257 { 258 struct device *dev = &pdev->dev; 259 struct mtk_ccifreq_drv *drv; 260 struct devfreq_passive_data *passive_data; 261 struct dev_pm_opp *opp; 262 unsigned long rate, opp_volt; 263 int ret; 264 265 drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); 266 if (!drv) 267 return -ENOMEM; 268 269 drv->dev = dev; 270 drv->soc_data = (const struct mtk_ccifreq_platform_data *) 271 of_device_get_match_data(&pdev->dev); 272 mutex_init(&drv->reg_lock); 273 platform_set_drvdata(pdev, drv); 274 275 drv->cci_clk = devm_clk_get(dev, "cci"); 276 if (IS_ERR(drv->cci_clk)) { 277 ret = PTR_ERR(drv->cci_clk); 278 return dev_err_probe(dev, ret, 279 "failed to get cci clk: %d\n", ret); 280 } 281 282 drv->inter_clk = devm_clk_get(dev, "intermediate"); 283 if (IS_ERR(drv->inter_clk)) { 284 ret = PTR_ERR(drv->inter_clk); 285 dev_err_probe(dev, ret, 286 "failed to get intermediate clk: %d\n", ret); 287 goto out_free_resources; 288 } 289 290 drv->proc_reg = devm_regulator_get_optional(dev, "proc"); 291 if (IS_ERR(drv->proc_reg)) { 292 ret = PTR_ERR(drv->proc_reg); 293 dev_err_probe(dev, ret, 294 "failed to get proc regulator: %d\n", ret); 295 goto out_free_resources; 296 } 297 298 ret = regulator_enable(drv->proc_reg); 299 if (ret) { 300 dev_err(dev, "failed to enable proc regulator\n"); 301 goto out_free_resources; 302 } 303 304 drv->sram_reg = regulator_get_optional(dev, "sram"); 305 if (IS_ERR(drv->sram_reg)) 306 drv->sram_reg = NULL; 307 else { 308 ret = regulator_enable(drv->sram_reg); 309 if (ret) { 310 dev_err(dev, "failed to enable sram regulator\n"); 311 goto out_free_resources; 312 } 313 } 314 315 /* 316 * We assume min voltage is 0 and tracking target voltage using 317 * min_volt_shift for each iteration. 318 * The retry_max is 3 times of expeted iteration count. 319 */ 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt, 321 drv->soc_data->proc_max_volt), 322 drv->soc_data->min_volt_shift); 323 324 ret = clk_prepare_enable(drv->cci_clk); 325 if (ret) 326 goto out_free_resources; 327 328 ret = clk_prepare_enable(drv->inter_clk); 329 if (ret) 330 goto out_disable_cci_clk; 331 332 ret = dev_pm_opp_of_add_table(dev); 333 if (ret) { 334 dev_err(dev, "failed to add opp table: %d\n", ret); 335 goto out_disable_inter_clk; 336 } 337 338 rate = clk_get_rate(drv->inter_clk); 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); 340 if (IS_ERR(opp)) { 341 ret = PTR_ERR(opp); 342 dev_err(dev, "failed to get intermediate opp: %d\n", ret); 343 goto out_remove_opp_table; 344 } 345 drv->inter_voltage = dev_pm_opp_get_voltage(opp); 346 dev_pm_opp_put(opp); 347 348 rate = U32_MAX; 349 opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); 350 if (IS_ERR(opp)) { 351 dev_err(dev, "failed to get opp\n"); 352 ret = PTR_ERR(opp); 353 goto out_remove_opp_table; 354 } 355 356 opp_volt = dev_pm_opp_get_voltage(opp); 357 dev_pm_opp_put(opp); 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); 359 if (ret) { 360 dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n", 361 opp_volt); 362 goto out_remove_opp_table; 363 } 364 365 passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data), 366 GFP_KERNEL); 367 if (!passive_data) { 368 ret = -ENOMEM; 369 goto out_remove_opp_table; 370 } 371 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; 373 drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile, 374 DEVFREQ_GOV_PASSIVE, 375 passive_data); 376 if (IS_ERR(drv->devfreq)) { 377 ret = -EPROBE_DEFER; > 378 dev_err(dev, "failed to add devfreq device: %d\n", 379 PTR_ERR(drv->devfreq)); 380 goto out_remove_opp_table; 381 } 382 383 drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; 384 ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); 385 if (ret) { 386 dev_err(dev, "failed to register opp notifier: %d\n", ret); 387 goto out_remove_devfreq_device; 388 } 389 return 0; 390 391 out_remove_devfreq_device: 392 devm_devfreq_remove_device(dev, drv->devfreq); 393 394 out_remove_opp_table: 395 dev_pm_opp_of_remove_table(dev); 396 397 out_disable_inter_clk: 398 clk_disable_unprepare(drv->inter_clk); 399 400 out_disable_cci_clk: 401 clk_disable_unprepare(drv->cci_clk); 402 403 out_free_resources: 404 if (regulator_is_enabled(drv->proc_reg)) 405 regulator_disable(drv->proc_reg); 406 if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) 407 regulator_disable(drv->sram_reg); 408 409 if (!IS_ERR(drv->proc_reg)) 410 regulator_put(drv->proc_reg); 411 if (!IS_ERR(drv->sram_reg)) 412 regulator_put(drv->sram_reg); 413 if (!IS_ERR(drv->cci_clk)) 414 clk_put(drv->cci_clk); 415 if (!IS_ERR(drv->inter_clk)) 416 clk_put(drv->inter_clk); 417 418 return ret; 419 } 420 -- 0-DAY CI Kernel Test Service https://01.org/lkp
© 2016 - 2024 Red Hat, Inc.