On 26/05/2020 11:12, Roger Lu wrote: > Hi Matthias, > > Thanks for the feedback. > > On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote: >> >> On 22/05/2020 11:40, Roger Lu wrote: >>> >>> Hi Enric, >>> >>> On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote: >>>> Hi Roger, >>>> >>>> Thank you for your patch. I have the feeling that this driver is >>>> complex and difficult to follow and I am wondering if it wouldn't be >>>> better if you can send a version that simply adds basic functionality >>>> for now. Some comments below. >>> >>> Thanks for the advices. I'll submit SVS v9 with basic functionality >>> patch + step by step functionalities' patches. >>> >>>> >>>> Missatge de Roger Lu <roger.lu@xxxxxxxxxxxx> del dia dl., 18 de maig >>>> 2020 a les 11:25: >>>>> >>>>> The SVS (Smart Voltage Scaling) engine is a piece >>>>> of hardware which is used to calculate optimized >>>>> voltage values of several power domains, >>>>> e.g. CPU/GPU/CCI, according to chip process corner, >>>>> temperatures, and other factors. Then DVFS driver >>>>> could apply those optimized voltage values to reduce >>>>> power consumption. >>>>> >>>>> Signed-off-by: Roger Lu <roger.lu@xxxxxxxxxxxx> >>>>> --- >>>>> drivers/power/avs/Kconfig | 10 + >>>>> drivers/power/avs/Makefile | 1 + >>>>> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++ >>>>> include/linux/power/mtk_svs.h | 23 + >>>>> 4 files changed, 2153 insertions(+) >>>>> create mode 100644 drivers/power/avs/mtk_svs.c >>>>> create mode 100644 include/linux/power/mtk_svs.h >>>>> >>>>> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig >>>>> index cdb4237bfd02..67089ac6040e 100644 >>>>> --- a/drivers/power/avs/Kconfig >>>>> +++ b/drivers/power/avs/Kconfig >>>>> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN >>>>> Say y here to enable support io domains on Rockchip SoCs. It is >>>>> necessary for the io domain setting of the SoC to match the >>>>> voltage supplied by the regulators. >>>>> + >>>>> +config MTK_SVS >>>>> + bool "MediaTek Smart Voltage Scaling(SVS)" >>>> >>>> Can't be this a module? Why? In such case, you should use tristate option >>> >>> Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide >>> module option in config. If, somehow, SVS isn't needed, we suggest >>> CONFIG_MTK_SVS=n to be set. >>> >> >> The question here is if it needs to be probed before we probe the modules. If >> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS. > > Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that > will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used > with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC > to enable SVS power feature. Anyway, back to Enric's question, I'll make > MTK_SVS become a tristate feature in the next patch. Thanks. > >> [...] >>>>> + >>>>> +static const u32 svs_regs_v2[] = { >>>> >>>> Is this SoC specific or shared between SoCs? >>> >>> Shared between SoCs. Some SVS in MTK SoCs use v2 register map. >>> >> >> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep? > > 1. MT8173 IC uses v1 register map. > 2. Yes, I'll keep v2 postfix. > Sounds good, thanks for clarification. Regards, Matthias