Re: [PATCH 3/4] clk/soc: mediatek: mt6779: Bind clock driver from platform device

From: Matthias Brugger <mbrugger@suse.com>
To: Chun-Kuang Hu <chunkuang.hu@kernel.org>, matthias.bgg@kernel.org
Cc: Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	devicetree@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	mtk01761 <wendell.lin@mediatek.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-clk@vger.kernel.org, Allison Randal <allison@lohutok.net>
Subject: Re: [PATCH 3/4] clk/soc: mediatek: mt6779: Bind clock driver from platform device
Date: Tue, 19 May 2020 23:26:34 +0200	[thread overview]
Message-ID: <3a8b3846-c399-2193-a203-8707d693dad8@suse.com> (raw)
In-Reply-To: <CAAOTY_80D6ZMM5_nEyf_XDjOLaWCOyi3mn9ibWhNX8_ozPK1BQ@mail.gmail.com>

On 19/05/2020 17:10, Chun-Kuang Hu wrote:
> Hi, Matthias:
> 
> <matthias.bgg@kernel.org> 於 2020年5月18日 週一 下午7:33寫道：
>>
>> From: Matthias Brugger <matthias.bgg@gmail.com>
>>
>> The mmsys driver is now the top level entry point for the multimedia
>> system (mmsys), we bind the clock driver by creating a platform device.
>> We also bind the MediaTek DRM driver which is not yet implement and
>> therefor will errror out for now.
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> ---
>>
>>  drivers/clk/mediatek/clk-mt6779-mm.c | 9 ++-------
>>  drivers/soc/mediatek/mtk-mmsys.c     | 8 ++++++++
>>  2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt6779-mm.c b/drivers/clk/mediatek/clk-mt6779-mm.c
>> index fb5fbb8e3e41..059c1a41ac7a 100644
>> --- a/drivers/clk/mediatek/clk-mt6779-mm.c
>> +++ b/drivers/clk/mediatek/clk-mt6779-mm.c
>> @@ -84,15 +84,11 @@ static const struct mtk_gate mm_clks[] = {
>>         GATE_MM1(CLK_MM_DISP_OVL_FBDC, "mm_disp_ovl_fbdc", "mm_sel", 16),
>>  };
>>
>> -static const struct of_device_id of_match_clk_mt6779_mm[] = {
>> -       { .compatible = "mediatek,mt6779-mmsys", },
>> -       {}
>> -};
>> -
>>  static int clk_mt6779_mm_probe(struct platform_device *pdev)
>>  {
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *node = dev->parent->of_node;
>>         struct clk_onecell_data *clk_data;
>> -       struct device_node *node = pdev->dev.of_node;
>>
>>         clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
>>
>> @@ -106,7 +102,6 @@ static struct platform_driver clk_mt6779_mm_drv = {
>>         .probe = clk_mt6779_mm_probe,
>>         .driver = {
>>                 .name = "clk-mt6779-mm",
>> -               .of_match_table = of_match_clk_mt6779_mm,
>>         },
>>  };
>>
>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
>> index fee64c8d3020..dc15808cf3a3 100644
>> --- a/drivers/soc/mediatek/mtk-mmsys.c
>> +++ b/drivers/soc/mediatek/mtk-mmsys.c
>> @@ -88,6 +88,10 @@ static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
>>         .clk_driver = "clk-mt2712-mm",
>>  };
>>
>> +static const struct mtk_mmsys_driver_data mt6779_mmsys_driver_data = {
>> +       .clk_driver = "clk-mt6779-mm",
> 
> This patch looks good to me, but I've one question: why drm driver
> just use single "mediatek-drm" for sub driver name, but clock driver
> has different name for each SoC?
> Could we just use single name for clock driver such as "mediatek-clk-mm"?

Because the DRM for all SoCs are handled in the same driver, while we have a
different clock drivers for each SoC. So we need to specify which clock driver
we want to load.

If we want to change that, we would need to refactor heavily the clock drivers
for all MediaTek SoCs. I don't think it's worth the effort.

Regards,
Matthias

> 
> Regards,
> Chun-Kuang.
> 
>> +};
>> +
>>  static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>>         .clk_driver = "clk-mt6797-mm",
>>  };
>> @@ -343,6 +347,10 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
>>                 .compatible = "mediatek,mt2712-mmsys",
>>                 .data = &mt2712_mmsys_driver_data,
>>         },
>> +       {
>> +               .compatible = "mediatek,mt6779-mmsys",
>> +               .data = &mt6779_mmsys_driver_data,
>> +       },
>>         {
>>                 .compatible = "mediatek,mt6797-mmsys",
>>                 .data = &mt6797_mmsys_driver_data,
>> --
>> 2.26.2
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek