Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine

[Matthias Brugger]

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