Hi Andrew-sh.Cheng,
Please add the dt-binding documentation.
On 19. 3. 29. 오후 3:46, Andrew-sh.Cheng wrote:
> This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
> of the Mediatek MT8183.
>
> On the MT8183 the CCI is supplied by the same regulator as the LITTLE
> cores. The driver is notified when the regulator voltage changes
> (driven by cpufreq) and adjusts the CCI frequency to the maximum
> possible value.
>
> Signed-off-by: Andrew-sh.Cheng
> ---
> drivers/devfreq/Kconfig | 10 ++
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/mt8183-cci-devfreq.c | 235
> +++
> 3 files changed, 246 insertions(+)
> create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 6a172d3..da2f8ec 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -91,6 +91,16 @@ config ARM_EXYNOS_BUS_DEVFREQ
> and adjusts the operating frequencies and voltages with OPP support.
> This does not yet operate with optimal voltages.
>
> +config ARM_MT8183_CCI_DEVFREQ
> + tristate "MT8183 CCI DEVFREQ Driver"
> + depends on ARM_MEDIATEK_CPUFREQ
> + help
> + This adds a devfreq driver for Cache Coherent Interconnect
> + of Mediatek MT8183, which is shared the same regulator
> + with cpu cluster.
> + It can track buck voltage and update a proper cci frequency.
> + Use notification to get regulator status.
> +
> config ARM_TEGRA_DEVFREQ
> tristate "Tegra DEVFREQ Driver"
> depends on ARCH_TEGRA_124_SOC
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 32b8d4d..817dde7 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
>
> # DEVFREQ Drivers
> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
> +obj-$(CONFIG_ARM_MT8183_CCI_DEVFREQ)+= mt8183-cci-devfreq.o
> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
>
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c
> b/drivers/devfreq/mt8183-cci-devfreq.c
> new file mode 100644
> index 000..af82d2e
> --- /dev/null
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -0,0 +1,235 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
You can add the author information under copyright information.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "governor.h"
> +
> +struct cci_devfreq {
> + struct devfreq *devfreq;
> + struct regulator*proc_reg;
> + unsigned long proc_reg_uV;
> + struct clk *cci_clk;
> + struct notifier_block nb;
Just use the space instead of tab as following:
struct devfreq *devfreq;
struct regulator *proc_reg;
unsigned long proc_reg_uV;
struct clk *cci_clk;
struct notifier_block nb;
> +};
> +
> +static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
> + unsigned long val, void *data)
> +{
> + struct cci_devfreq *cci_df =
> + container_of(nb, struct cci_devfreq, nb);
> +
> + /* deal with reduce frequency */
> + if (val & REGULATOR_EVENT_PRE_VOLTAGE_CHANGE) {
> + struct pre_voltage_change_data *pvc_data = data;
> +
> + if (pvc_data->min_uV < pvc_data->old_uV) {
> + cci_df->proc_reg_uV =
> + (unsigned long)(pvc_data->min_uV);
> + mutex_lock(_df->devfreq->lock);
> + update_devfreq(cci_df->devfreq);
Please handle the return value of update_devfreq() for exception handling.
> + mutex_unlock(_df->devfreq->lock);
> + }
> + }
> +
> + if ((val & REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE) &&
if -> else if
At the same time, receives only one notification.
It is not necessary to check the two if statement always, .
> + ((unsigned long)data > cci_df->proc_reg_uV)) {
> + cci_df->proc_reg_uV = (unsigned long)data;
> + mutex_lock(_df->devfreq->lock);
> + update_devfreq(cci_df->devfreq);
ditto. Please check the return value of update_devfreq.
> + mutex_unlock(_df->devfreq->lock);
> + }
> +
> + /* deal with increase frequency */
> + if ((val & REGULATOR_EVENT_VOLTAGE_CHANGE) &&
ditto.
if -> else if
At the same time, receives only one notification.
It is not necessary to check the two if statement always, .
> + (cci_df->proc_reg_uV < (unsigned long)data)) {
> + cci_df->proc_reg_uV = (unsigned long)data;
> + mutex_lock(_df->devfreq->lock);
> + update_devfreq(cci_df->devfreq);
ditto. Please
