Re: [PATCH 08/12] soc: mediatek: pm-domains: Add subsystem clocks

Matthias Brugger
2020-09-25T12:20:31Z

On 25/09/2020 12:55, Weiyi Lu wrote:
> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> For the bus protection operations, some subsystem clocks need to be enabled
>> before releasing the protection. This patch identifies the subsystem clocks
>> by it's name.
>>
>> Suggested-by: Weiyi Lu <weiyi.lu@mediatek.com>
>> [Adapted the patch to the mtk-pm-domains driver]
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> drivers/soc/mediatek/mtk-pm-domains.c | 82 +++++++++++++++++++++++----
>> 1 file changed, 70 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
>> index 0802eccc3a0b..52a93a87e313 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[...]
>>
>> - pd->num_clks = of_clk_get_parent_count(node);
>> - if (pd->num_clks > 0) {
>> + num_clks = of_clk_get_parent_count(node);
>> + if (num_clks > 0) {
>> + /* Calculate number of subsys_clks */
>> + of_property_for_each_string(node, "clock-names", prop, clk_name) {
>> + char *subsys;
>> +
>> + subsys = strchr(clk_name, '-');
>> + if (subsys)
>> + pd->num_subsys_clks++;
>> + else
>> + pd->num_clks++;
>> + }
>> +
>
> In fact, I don't like the clock naming rules, as Matthias mentioned
> before. So in my work v17[1]
> I put subsystem clocks under each power domain sub-node without giving
> the clock name but put the basic clocks under the power controller node.
>
> [1] https://patchwork.kernel.org/patch/11703191/

The quick answer, there is no perfect solution to our problem. If we put all
basic clocks under the parent node, then we will have to have a list of basic
clocks in the driver data for each SoC. Apart from that the DTS would not
reflect the exact hardware, as the basic clocks needed for every power domain
would be hidden in the driver data.

Given this, I think the best way is to distinguish the clocks by it's name, as
done by you in earlier series of the scpsys. It will give us a cleaner device
tree and we won't need to add long clock lists in the driver data. If I remember
correctly Rob acknowledged your binding change for that:
https://patchwork.kernel.org/patch/11562521/

Regards,
Matthias