Re: [PATCH v21 3/8] soc: mediatek: SVS: introduce MTK SVS engine

[AngeloGioacchino Del Regno]

Il 07/01/22 10:51, Roger Lu ha scritto:
> The Smart Voltage Scaling(SVS) engine is a piece of hardware
> which calculates suitable SVS bank voltages to OPP voltage table.
> Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck
> when receiving OPP_EVENT_ADJUST_VOLTAGE.
>
> Signed-off-by: Roger Lu <roger.lu@xxxxxxxxxxxx>
> ---
>   drivers/soc/mediatek/Kconfig   |   10 +
>   drivers/soc/mediatek/Makefile  |    1 +
>   drivers/soc/mediatek/mtk-svs.c | 1446 ++++++++++++++++++++++++++++++++
>   3 files changed, 1457 insertions(+)
>   create mode 100644 drivers/soc/mediatek/mtk-svs.c
>
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index fdd8bc08569e..3c3eedea35f7 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -73,4 +73,14 @@ config MTK_MMSYS
>   	  Say yes here to add support for the MediaTek Multimedia
>   	  Subsystem (MMSYS).
>   
> +config MTK_SVS
> +	tristate "MediaTek Smart Voltage Scaling(SVS)"
> +	depends on MTK_EFUSE && NVMEM
> +	help
> +	  The Smart Voltage Scaling(SVS) engine is a piece of hardware
> +	  which has several controllers(banks) for calculating suitable
> +	  voltage to different power domains(CPU/GPU/CCI) according to
> +	  chip process corner, temperatures and other factors. Then DVFS
> +	  driver could apply SVS bank voltage to PMIC/Buck.
> +
>   endmenu
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 90270f8114ed..0e9e703c931a 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
>   obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
>   obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
>   obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
> +obj-$(CONFIG_MTK_SVS) += mtk-svs.o
> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> new file mode 100644
> index 000000000000..fc7e2ee44a92
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-svs.c
> @@ -0,0 +1,1446 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_opp.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +/* svs bank 1-line sw id */
> +#define SVSB_CPU_LITTLE			BIT(0)
> +#define SVSB_CPU_BIG			BIT(1)
> +#define SVSB_CCI			BIT(2)
> +#define SVSB_GPU			BIT(3)
> +
> +/* svs bank mode support */
> +#define SVSB_MODE_ALL_DISABLE		0
> +#define SVSB_MODE_INIT01		BIT(1)
> +#define SVSB_MODE_INIT02		BIT(2)
> +
> +/* svs bank volt flags */
> +#define SVSB_INIT01_PD_REQ		BIT(0)
> +#define SVSB_INIT01_VOLT_IGNORE		BIT(1)
> +#define SVSB_INIT01_VOLT_INC_ONLY	BIT(2)
> +
> +/* svs bank common setting */
> +#define MAX_OPP_ENTRIES			16
> +#define SVSB_DC_SIGNED_BIT		BIT(15)
> +#define SVSB_DET_CLK_EN			BIT(31)
> +#define SVSB_DET_MAX			0xffff
> +#define SVSB_DET_WINDOW			0xa28
> +#define SVSB_DTHI			0x1
> +#define SVSB_DTLO			0xfe
> +#define SVSB_EN_INIT01			0x1
> +#define SVSB_EN_INIT02			0x5
> +#define SVSB_EN_MASK			0x7
> +#define SVSB_EN_OFF			0x0
> +#define SVSB_INTEN_INIT0x		0x00005f01
> +#define SVSB_INTSTS_CLEAN		0x00ffffff
> +#define SVSB_INTSTS_COMPLETE		0x1
> +#define SVSB_RUNCONFIG_DEFAULT		0x80000000
> +
> +static DEFINE_SPINLOCK(svs_lock);

snip....


> +
> +struct svs_platform_data {
> +	char *name;
> +	struct svs_bank *banks;
> +	bool (*efuse_parsing)(struct svs_platform *svsp);
> +	unsigned long irqflags;
> +	const u32 *regs;
> +	u32 bank_max;
> +	int (*probe)(struct svs_platform *svsp);
> +};
> +

Please move the definition of struct svs_platform_data at the beginning of
the file for increased readability.

With that done,

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>