| Message ID | 20210704024032.11559-3-shawn.guo@linaro.org |
|---|---|
| State | Accepted |
| Commit | 05cc560c8cb4c2fe39022c1f397125470b28705c |
| Headers | show |
| Series | Add MSM8939 APCS/A53PLL clock support | expand |
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 >
On Sat, Jul 10, 2021 at 12:17:33AM -0500, Bjorn Andersson wrote: > 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) ? dev_name() is somehow reasonable for a53pll. b016000.clock-controller:a53pll b116000.clock-controller:a53pll b1d0000.clock-controller:a53pll But I prefer to the existing names, because I would like to use the same naming schema for both a53pll and a53mux. If using dev_name() on a53mux, we will get the following which is less reasonable. qcom-apcs-msm8916-clk.1.auto:a53mux qcom-apcs-msm8916-clk.2.auto:a53mux qcom-apcs-msm8916-clk.3.auto:a53mux Shawn
Quoting Shawn Guo (2021-07-03 19:40:30) > 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> > --- Applied to clk-next
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, '@')); + 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;
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(-)