Message ID | 20210704024032.11559-1-shawn.guo@linaro.org |
---|---|
Headers | show |
Series | Add MSM8939 APCS/A53PLL clock support | expand |
Le dimanche 04 juillet 2021 à 10:40 +0800, Shawn Guo a écrit : > This series adds MSM8939 APCS/A53PLL clock support. Most outstanding > thing about MSM8939 is that it integrates 3 APCS instances, for Cluster0 > (little cores), Cluster1 (big cores) and CCI (Cache Coherent Interconnect) > respectively. > > Changes for v2: > - Reword the commit log of first patch as suggested by Stephen. > - Drop 'clock-output-names' bindings and use @unit-address to get unique > a53pll/mux clock names. > - Use 'operating-points-v2' bindings to pass frequency table via OPP, so > that we can use one single compatible for all 3 MSM8939 a53pll. > > Shawn Guo (4): > clk: qcom: apcs-msm8916: Flag a53mux instead of a53pll as critical > clk: qcom: a53pll/mux: Use unique clock name > dt-bindings: clock: Update qcom,a53pll bindings for MSM8939 support > clk: qcom: a53-pll: Add MSM8939 a53pll support > > .../bindings/clock/qcom,a53pll.yaml | 3 + > drivers/clk/qcom/a53-pll.c | 68 ++++++++++++++++++- > drivers/clk/qcom/apcs-msm8916.c | 10 ++- > 3 files changed, 76 insertions(+), 5 deletions(-) Hello, would you have a msm8939 dtsi/dts reference file working with all recent contributions for this SoC ? We the msm8939-focused PostmarketOS gang would be happy to boot our devices and test patches but we're not able to boot anything more recent that 5.9...
On Wed, Jul 07, 2021 at 11:34:19PM +0200, Vincent Knecht wrote: > Le dimanche 04 juillet 2021 à 10:40 +0800, Shawn Guo a écrit : > > This series adds MSM8939 APCS/A53PLL clock support. Most outstanding > > thing about MSM8939 is that it integrates 3 APCS instances, for Cluster0 > > (little cores), Cluster1 (big cores) and CCI (Cache Coherent Interconnect) > > respectively. > > > > Changes for v2: > > - Reword the commit log of first patch as suggested by Stephen. > > - Drop 'clock-output-names' bindings and use @unit-address to get unique > > a53pll/mux clock names. > > - Use 'operating-points-v2' bindings to pass frequency table via OPP, so > > that we can use one single compatible for all 3 MSM8939 a53pll. > > > > Shawn Guo (4): > > clk: qcom: apcs-msm8916: Flag a53mux instead of a53pll as critical > > clk: qcom: a53pll/mux: Use unique clock name > > dt-bindings: clock: Update qcom,a53pll bindings for MSM8939 support > > clk: qcom: a53-pll: Add MSM8939 a53pll support > > > > .../bindings/clock/qcom,a53pll.yaml | 3 + > > drivers/clk/qcom/a53-pll.c | 68 ++++++++++++++++++- > > drivers/clk/qcom/apcs-msm8916.c | 10 ++- > > 3 files changed, 76 insertions(+), 5 deletions(-) > > Hello, > > would you have a msm8939 dtsi/dts reference file working with all recent > contributions for this SoC ? Yes, the dts will be posted once it's ready for public review. Shawn > We the msm8939-focused PostmarketOS gang would be happy to boot our devices > and test patches but we're not able to boot anything more recent that 5.9...
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
On Sun, Jul 04, 2021 at 10:40:28AM +0800, Shawn Guo wrote: > This series adds MSM8939 APCS/A53PLL clock support. Most outstanding > thing about MSM8939 is that it integrates 3 APCS instances, for Cluster0 > (little cores), Cluster1 (big cores) and CCI (Cache Coherent Interconnect) > respectively. > > Changes for v2: > - Reword the commit log of first patch as suggested by Stephen. > - Drop 'clock-output-names' bindings and use @unit-address to get unique > a53pll/mux clock names. > - Use 'operating-points-v2' bindings to pass frequency table via OPP, so > that we can use one single compatible for all 3 MSM8939 a53pll. Hi Stephen, Any comments on this version? Shawn
Quoting Shawn Guo (2021-07-03 19:40:29) > The clock source for MSM8916 cpu cores is like below. > > |\ > a53pll --------| \ a53mux +------+ Applied to clk-next
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
Quoting Shawn Guo (2021-07-03 19:40:32) > MSM8939 has 3 a53pll clocks with different frequency table for Cluster0, > Cluster1 and CCI. It adds function qcom_a53pll_get_freq_tbl() to create > pll_freq_tbl from OPP, so that those a53pll frequencies can be defined > in DT with operating-points-v2 bindings rather than being coded in the > driver. In this case, one compatible rather than three would be needed > for these 3 a53pll clocks. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > --- Applied to clk-next