Re: [PATCH V6 08/13] interconnect: mediatek: Add interconnect provider driver

From: Georgi Djakov
Date: Wed Jan 06 2021 - 07:54:26 EST


On 6.01.21 12:44, HenryC Chen (陳建豪) wrote:
-----Original Message-----
From: Georgi Djakov [mailto:georgi.djakov@xxxxxxxxxx]
Sent: Wednesday, January 06, 2021 4:14 PM
To: HenryC Chen (陳建豪)
Cc: Rob Herring; Matthias Brugger; Stephen Boyd; Ryan Case; Mark Brown; Mark
Rutland; Nicolas Boichat; Fan Chen (陳凡); JamesJJ Liao (廖建智); Arvin Wang
(王志銘); devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
linux-mediatek@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
pm@xxxxxxxxxxxxxxx
Subject: Re: [PATCH V6 08/13] interconnect: mediatek: Add interconnect
provider driver

On 1/6/21 09:30, Henry Chen wrote:
> On Mon, 2021-01-04 at 20:36 +0200, Georgi Djakov wrote:
>> On 12/24/20 08:08, Henry Chen wrote:
>>> Introduce Mediatek MT6873/MT8183/MT8192 specific provider driver
>>> using the interconnect framework.
>>>
>>> ICC provider ICC Nodes
>>> ---- ----
>>> --------- |CPU | |--- |VPU |
>>> ----- | |----- ---- | ----
>>> |DRAM |--|DRAM | ---- | ----
>>> | |--|scheduler|----- |GPU | |--- |DISP|
>>> | |--|(EMI) | ---- | ----
>>> | |--| | ----- | ----
>>> ----- | |----- |MMSYS|--|--- |VDEC|
>>> --------- ----- | ----
>>> /|\ | ----
>>> |change DRAM freq |--- |VENC|
>>> ---------- | ----
>>> | DVFSR | |
>>> | | | ----
>>> ---------- |--- |IMG |
>>> | ----
>>> | ----
>>> |--- |CAM |
>>> ----
>>>
>>> Signed-off-by: Henry Chen <henryc.chen@xxxxxxxxxxxx>
>>> ---
>>> drivers/interconnect/Kconfig | 1 +
>>> drivers/interconnect/Makefile | 1 +
>>> drivers/interconnect/mediatek/Kconfig | 13 ++
>>> drivers/interconnect/mediatek/Makefile | 3 +
>>> drivers/interconnect/mediatek/mtk-emi.c | 330
++++++++++++++++++++++++++++++++
>>> 5 files changed, 348 insertions(+)
>>> create mode 100644 drivers/interconnect/mediatek/Kconfig
>>> create mode 100644 drivers/interconnect/mediatek/Makefile
>>> create mode 100644 drivers/interconnect/mediatek/mtk-emi.c
>>>
>>> diff --git a/drivers/interconnect/Kconfig
>>> b/drivers/interconnect/Kconfig index 5b7204e..e939f5a 100644
>>> --- a/drivers/interconnect/Kconfig
>>> +++ b/drivers/interconnect/Kconfig
>>> @@ -13,5 +13,6 @@ if INTERCONNECT
>>>
>>> source "drivers/interconnect/imx/Kconfig"
>>> source "drivers/interconnect/qcom/Kconfig"
>>> +source "drivers/interconnect/mediatek/Kconfig"
>>
>> Sort alphabetically please.
> Ok
>>
>>>
>>> endif
>>> diff --git a/drivers/interconnect/Makefile
>>> b/drivers/interconnect/Makefile index d203520..0643a24 100644
>>> --- a/drivers/interconnect/Makefile
>>> +++ b/drivers/interconnect/Makefile
>>> @@ -6,3 +6,4 @@ icc-core-objs:= core.o bulk.o
>>> obj-$(CONFIG_INTERCONNECT)+= icc-core.o
>>> obj-$(CONFIG_INTERCONNECT_IMX)+= imx/
>>> obj-$(CONFIG_INTERCONNECT_QCOM)+= qcom/
>>> +obj-$(CONFIG_INTERCONNECT_MTK)+= mediatek/
>>
>> Ditto.
> Ok
>>
>>> diff --git a/drivers/interconnect/mediatek/Kconfig
>>> b/drivers/interconnect/mediatek/Kconfig
>>> new file mode 100644
>>> index 0000000..972d3bb
>>> --- /dev/null
>>> +++ b/drivers/interconnect/mediatek/Kconfig
>>> @@ -0,0 +1,13 @@
>>> +config INTERCONNECT_MTK
>>> +bool "Mediatek Network-on-Chip interconnect drivers"
>>> +depends on ARCH_MEDIATEK
>>> +help
>>> + Support for Mediatek's Network-on-Chip interconnect hardware.
>>> +
>>> +config INTERCONNECT_MTK_EMI
>>> +tristate "Mediatek EMI interconnect driver"
>>> +depends on INTERCONNECT_MTK
>>> +depends on (MTK_DVFSRC && OF)
>>
>> Would it be possible to enable COMPILE_TEST?
> Do you means change to "depends on (MTK_DVFSRC && OF) || COMPILE_TEST" ?

Yeah, there is a stub for mtk_dvfsrc_send_request(). Maybe we could even
change it for INTERCONNECT_MTK to something like:
depends on ARCH_MEDIATEK || COMPILE_TEST Will this work?
Hi Georgi,

So..only change to as following?

config INTERCONNECT_MTK
bool "Mediatek Network-on-Chip interconnect drivers"
depends on ARCH_MEDIATEK || COMPILE_TEST
help
Support for Mediatek's Network-on-Chip interconnect hardware.

I would say to add COMPILE_TEST for both INTERCONNECT_MTK and
INTERCONNECT_MTK_EMI, unless there is some dependency which does
not allow us to do so.

Thanks,
Georgi