Re: [PATCH v2 2/4] clk: qcom: a53pll/mux: Use unique clock name

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Shawn Guo <shawn.guo@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Sivaprakash Murugesan <sivaprak@codeaurora.org>,
	Benjamin Li <benl@squareup.com>,
	devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-clk@vger.kernel.org
Subject: Re: [PATCH v2 2/4] clk: qcom: a53pll/mux: Use unique clock name
Date: Sat, 10 Jul 2021 00:17:33 -0500	[thread overview]
Message-ID: <YOktbWmMKEw7g3+E@yoga> (raw)
In-Reply-To: <20210704024032.11559-3-shawn.guo@linaro.org>

On Sat 03 Jul 21:40 CDT 2021, Shawn Guo wrote:

> Different from MSM8916 which has only one a53pll/mux clock, MSM8939 gets
> three for Cluster0 (little cores), Cluster1 (big cores) and CCI (Cache
> Coherent Interconnect).  That said, a53pll/mux clock needs to be named
> uniquely.  Append @unit-address of device node to the clock name, so
> that a53pll/mux will be named like below on MSM8939.
> 
>   a53pll@b016000
>   a53pll@b116000
>   a53pll@b1d0000
> 
>   a53mux@b1d1000
>   a53mux@b011000
>   a53mux@b111000
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/clk/qcom/a53-pll.c      | 8 +++++++-
>  drivers/clk/qcom/apcs-msm8916.c | 8 +++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> index d6756bd777ce..96a118be912d 100644
> --- a/drivers/clk/qcom/a53-pll.c
> +++ b/drivers/clk/qcom/a53-pll.c
> @@ -37,6 +37,7 @@ static const struct regmap_config a53pll_regmap_config = {
>  static int qcom_a53pll_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
>  	struct regmap *regmap;
>  	struct resource *res;
>  	struct clk_pll *pll;
> @@ -66,7 +67,12 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>  	pll->status_bit = 16;
>  	pll->freq_tbl = a53pll_freq;
>  
> -	init.name = "a53pll";
> +	/* Use an unique name by appending @unit-address */
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "a53pll%s",
> +				   strchrnul(np->full_name, '@'));

While the result is nice, this isn't...

Is your dev_name() reasonable? What about "%s:a53pll", dev_name(dev) ?

Regards,
Bjorn

> +	if (!init.name)
> +		return -ENOMEM;
> +
>  	init.parent_names = (const char *[]){ "xo" };
>  	init.num_parents = 1;
>  	init.ops = &clk_pll_sr2_ops;
> diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
> index d7ac6d6b15b6..89e0730810ac 100644
> --- a/drivers/clk/qcom/apcs-msm8916.c
> +++ b/drivers/clk/qcom/apcs-msm8916.c
> @@ -46,6 +46,7 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct device *parent = dev->parent;
> +	struct device_node *np = parent->of_node;
>  	struct clk_regmap_mux_div *a53cc;
>  	struct regmap *regmap;
>  	struct clk_init_data init = { };
> @@ -61,7 +62,12 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
>  	if (!a53cc)
>  		return -ENOMEM;
>  
> -	init.name = "a53mux";
> +	/* Use an unique name by appending parent's @unit-address */
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "a53mux%s",
> +				   strchrnul(np->full_name, '@'));
> +	if (!init.name)
> +		return -ENOMEM;
> +
>  	init.parent_data = pdata;
>  	init.num_parents = ARRAY_SIZE(pdata);
>  	init.ops = &clk_regmap_mux_div_ops;
> -- 
> 2.17.1
> 