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

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <923ec8f9-32fe-5a1c-e7fe-8dc1c55d664c@suse.com>
Date:   Fri, 25 Sep 2020 14:20:25 +0200
From:   Matthias Brugger <mbrugger@...e.com>
To:     Weiyi Lu <weiyi.lu@...iatek.com>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     linux-kernel@...r.kernel.org,
        Collabora Kernel ML <kernel@...labora.com>,
        fparent@...libre.com, matthias.bgg@...il.com,
        drinkcat@...omium.org, hsinyi@...omium.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 08/12] soc: mediatek: pm-domains: Add subsystem clocks



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@...e.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@...iatek.com>
>> [Adapted the patch to the mtk-pm-domains driver]
>> Signed-off-by: Matthias Brugger <mbrugger@...e.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.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