Hi Chun-Jie,
On 09/06/2021 00:38, Chun-Jie Chen wrote:
> On Tue, 2021-06-08 at 16:44 +0200, Matthias Brugger wrote:
>>
>> On 24/05/2021 14:20, Chun-Jie Chen wrote:
>>> Add MT8192 mmsys clock provider
>>>
>>> Signed-off-by: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>
>>> Signed-off-by: chun-jie.chen <chun-jie.chen@xxxxxxxxxxxx>
>>> ---
>>> drivers/clk/mediatek/Kconfig | 6 ++
>>> drivers/clk/mediatek/Makefile | 1 +
>>> drivers/clk/mediatek/clk-mt8192-mm.c | 108
>>> +++++++++++++++++++++++++++
>>> 3 files changed, 115 insertions(+)
>>> create mode 100644 drivers/clk/mediatek/clk-mt8192-mm.c
>>>
>>
>> [...]
>>> +
>>> +static int clk_mt8192_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;
>>> + int r;
>>> +
>>> + clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
>>> + if (!clk_data)
>>> + return -ENOMEM;
>>> +
>>> + r = mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
>>> clk_data);
>>> + if (r)
>>> + return r;
>>> +
>>> + return of_clk_add_provider(node, of_clk_src_onecell_get,
>>> clk_data);
>>> +}
>>> +
>>> +static struct platform_driver clk_mt8192_mm_drv = {
>>> + .probe = clk_mt8192_mm_probe,
>>> + .driver = {
>>> + .name = "clk-mt8192-mm",
>>> + },
>>> +};
>>
>> Did you had a look at drivers/soc/mediatek/mtk-mmsys.c? How is the
>> MMSYS
>> different from all the other SoCs? I suppose it is not. Please don't
>> just
>> implement the clock drivers, but check in existing code how they play
>> together
>> with the HW they are for. MediaTek unfortunately has the design to
>> add the clock
>> registers in the address space of the IP block that needs this
>> registers. Which
>> makes it more complicated to implement clock driver in the first
>> place.
>>
>> Regards,
>> Matthias
>
> Did you means binding the mm clock driver by creating a platform device
> in drivers/soc/mediatek/mtk-mmsys.c? There is 8192 mmsys compatible
> data in patch [1] but lack of it in the latest patch [2], I will check
> it.
> Thanks for your kind reminder.
>
Yes, the clock driver should be a platform driver. Binding should be done
through the soc driver.
Thanks a lot,
Matthias
> [1]
> https://patchwork.kernel.org/project/linux-mediatek/patch/1609815993-22744-11-git-send-email-yongqiang.niu@xxxxxxxxxxxx/
> [2]
> https://patchwork.kernel.org/project/linux-mediatek/patch/1618236288-1617-5-git-send-email-yongqiang.niu@xxxxxxxxxxxx/
>
> Best Regards,
> Chun-Jie
>
