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

[Matthias Brugger]

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