linux-kernel - Re: [PATCH v9 18/22] clk: mediatek: Add MT8192 mmsys clock support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <b7a1c230-3f09-d9be-1138-3a5fa87600e7@gmail.com>
Date:   Wed, 9 Jun 2021 10:08:40 +0200
From:   Matthias Brugger <matthias.bgg@...il.com>
To:     Chun-Jie Chen <chun-jie.chen@...iatek.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Nicolas Boichat <drinkcat@...omium.org>,
        Rob Herring <robh+dt@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mediatek@...ts.infradead.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, srv_heupstream@...iatek.com,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        Weiyi Lu <weiyi.lu@...iatek.com>
Subject: Re: [PATCH v9 18/22] clk: mediatek: Add MT8192 mmsys clock support

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@...iatek.com>
>>> Signed-off-by: chun-jie.chen <chun-jie.chen@...iatek.com>
>>> ---
>>>  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@mediatek.com/
> [2] 
> https://patchwork.kernel.org/project/linux-mediatek/patch/1618236288-1617-5-git-send-email-yongqiang.niu@mediatek.com/
> 
> Best Regards,
> Chun-Jie
>