| Message ID | 20230120061417.2623751-5-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: qcom: msm8996: add support for the CBF clock | expand |
On 20.01.2023 07:14, Dmitry Baryshkov wrote: > Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth) > according to CPU frequencies. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/clk/qcom/clk-cbf-8996.c | 143 +++++++++++++++++++++++++++++++- > 1 file changed, 142 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c > index 9cde0e660228..b049b4f7b270 100644 > --- a/drivers/clk/qcom/clk-cbf-8996.c > +++ b/drivers/clk/qcom/clk-cbf-8996.c > @@ -5,11 +5,14 @@ > #include <linux/bitfield.h> > #include <linux/clk.h> > #include <linux/clk-provider.h> > +#include <linux/interconnect-provider.h> > #include <linux/of.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > > +#include <dt-bindings/interconnect/qcom,msm8996-cbf.h> > + > #include "clk-alpha-pll.h" > #include "clk-regmap.h" > > @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { > .val_format_endian = REGMAP_ENDIAN_LITTLE, > }; > > +#ifdef CONFIG_INTERCONNECT > +struct qcom_msm8996_cbf_icc_provider { > + struct icc_provider provider; > + struct clk *clk; > +}; > + > +#define to_qcom_cbf_provider(_provider) \ > + container_of(_provider, struct qcom_msm8996_cbf_icc_provider, provider) > + > +enum { > + CBF_MASTER_NODE = 2000, > + CBF_SLAVE_NODE > +}; > + > +#define CBF_NUM_NODES 2 > + > +static int qcom_msm8996_cbf_set(struct icc_node *src, struct icc_node *dst) > +{ > + struct qcom_msm8996_cbf_icc_provider *qp; > + > + qp = to_qcom_cbf_provider(src->provider); > + > + return clk_set_rate(qp->clk, icc_units_to_bps(dst->peak_bw)); > +} > + > +static int qcom_msm8996_cbf_icc_get_bw(struct icc_node *node, u32 *avg, u32 *peak) > +{ > + struct qcom_msm8996_cbf_icc_provider *qp; > + > + qp = to_qcom_cbf_provider(node->provider); > + *peak = clk_get_rate(qp->clk) / 1000ULL; > + > + return 0; > +} > + > +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw) > +{ > + struct device *dev = &pdev->dev; > + struct qcom_msm8996_cbf_icc_provider *qp; > + struct icc_provider *provider; > + struct icc_onecell_data *data; > + struct icc_node *node; > + struct clk *clk; > + int ret; > + > + clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + data = devm_kzalloc(dev, struct_size(data, nodes, CBF_NUM_NODES), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->num_nodes = CBF_NUM_NODES; > + > + qp = devm_kzalloc(dev, sizeof(*qp), GFP_KERNEL); > + if (!qp) > + return -ENOMEM; > + > + qp->clk = clk; > + > + provider = &qp->provider; > + provider->dev = dev; > + provider->get_bw = qcom_msm8996_cbf_icc_get_bw; > + provider->set = qcom_msm8996_cbf_set; > + provider->aggregate = icc_std_aggregate; > + provider->xlate = of_icc_xlate_onecell; > + INIT_LIST_HEAD(&provider->nodes); > + provider->data = data; > + > + ret = icc_provider_add(provider); > + if (ret) { > + dev_err(dev, "error adding interconnect provider\n"); > + return ret; > + } > + > + node = icc_node_create(CBF_MASTER_NODE); > + if (IS_ERR(node)) { > + ret = PTR_ERR(node); > + goto err; > + } > + > + node->name = "cbf_master"; > + icc_node_add(node, provider); > + icc_link_create(node, CBF_SLAVE_NODE); > + data->nodes[MASTER_CBF_M4M] = node; > + > + node = icc_node_create(CBF_SLAVE_NODE); > + if (IS_ERR(node)) { > + ret = PTR_ERR(node); > + goto err; > + } > + > + node->name = "cbf_slave"; > + icc_node_add(node, provider); > + data->nodes[SLAVE_CBF_M4M] = node; > + > + platform_set_drvdata(pdev, provider); > + > + return 0; > + > +err: > + icc_nodes_remove(provider); > + icc_provider_del(provider); > + > + return ret; > +} > + > +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev) > +{ > + struct icc_provider *provider = platform_get_drvdata(pdev); > + > + icc_nodes_remove(provider); > + icc_provider_del(provider); > + > + return 0; > +} > +#else > +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev) > +{ > + dev_warn(&pdev->dev, "interconnects support is disabled, CBF clock is fixed\n"); > + > + return 0; > +} > +#define qcom_msm8996_cbf_icc_remove(pdev) (0) > +#endif > + > static int qcom_msm8996_cbf_probe(struct platform_device *pdev) > { > void __iomem *base; > @@ -283,7 +413,16 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev) > if (ret) > return ret; > > - return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw); > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw); > + if (ret) > + return ret; > + > + return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw); > +} > + > +static int qcom_msm8996_cbf_remove(struct platform_device *pdev) > +{ > + return qcom_msm8996_cbf_icc_remove(pdev); > } > > static const struct of_device_id qcom_msm8996_cbf_match_table[] = { > @@ -294,9 +433,11 @@ MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table); > > static struct platform_driver qcom_msm8996_cbf_driver = { > .probe = qcom_msm8996_cbf_probe, > + .remove = qcom_msm8996_cbf_remove, > .driver = { > .name = "qcom-msm8996-cbf", > .of_match_table = qcom_msm8996_cbf_match_table, > + .sync_state = icc_sync_state, > }, > }; >
Quoting Dmitry Baryshkov (2023-01-19 22:14:13) > Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth) > according to CPU frequencies. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/clk/qcom/clk-cbf-8996.c | 143 +++++++++++++++++++++++++++++++- > 1 file changed, 142 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c > index 9cde0e660228..b049b4f7b270 100644 > --- a/drivers/clk/qcom/clk-cbf-8996.c > +++ b/drivers/clk/qcom/clk-cbf-8996.c > @@ -5,11 +5,14 @@ > #include <linux/bitfield.h> > #include <linux/clk.h> > #include <linux/clk-provider.h> > +#include <linux/interconnect-provider.h> > #include <linux/of.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > > +#include <dt-bindings/interconnect/qcom,msm8996-cbf.h> > + > #include "clk-alpha-pll.h" > #include "clk-regmap.h" > > @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { > .val_format_endian = REGMAP_ENDIAN_LITTLE, > }; > > +#ifdef CONFIG_INTERCONNECT Can you move this driver to drivers/interconnect/ ? > +struct qcom_msm8996_cbf_icc_provider { > + struct icc_provider provider; > + struct clk *clk; > +}; > + > +#define to_qcom_cbf_provider(_provider) \ > + container_of(_provider, struct qcom_msm8996_cbf_icc_provider, provider) > + > +enum { > + CBF_MASTER_NODE = 2000, [...] > +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev) > +{ > + struct icc_provider *provider = platform_get_drvdata(pdev); > + > + icc_nodes_remove(provider); > + icc_provider_del(provider); > + > + return 0; > +} > +#else > +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev) > +{ > + dev_warn(&pdev->dev, "interconnects support is disabled, CBF clock is fixed\n"); > + > + return 0; > +} > +#define qcom_msm8996_cbf_icc_remove(pdev) (0) It's like two drivers in one. > +#endif > + > static int qcom_msm8996_cbf_probe(struct platform_device *pdev) > { > void __iomem *base;
On 21/01/2023 00:11, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2023-01-19 22:14:13) >> Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth) >> according to CPU frequencies. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/clk/qcom/clk-cbf-8996.c | 143 +++++++++++++++++++++++++++++++- >> 1 file changed, 142 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c >> index 9cde0e660228..b049b4f7b270 100644 >> --- a/drivers/clk/qcom/clk-cbf-8996.c >> +++ b/drivers/clk/qcom/clk-cbf-8996.c >> @@ -5,11 +5,14 @@ >> #include <linux/bitfield.h> >> #include <linux/clk.h> >> #include <linux/clk-provider.h> >> +#include <linux/interconnect-provider.h> >> #include <linux/of.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> #include <linux/regmap.h> >> >> +#include <dt-bindings/interconnect/qcom,msm8996-cbf.h> >> + >> #include "clk-alpha-pll.h" >> #include "clk-regmap.h" >> >> @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { >> .val_format_endian = REGMAP_ENDIAN_LITTLE, >> }; >> >> +#ifdef CONFIG_INTERCONNECT > > Can you move this driver to drivers/interconnect/ ? Only the interconnect part? At some point I considered dropping the whole CBF mux support and moving the whole driver to drivers/interconnect, but I could not find a good way to use alpha-pll from the interconnect driver. Would you recommend one? > >> +struct qcom_msm8996_cbf_icc_provider { >> + struct icc_provider provider; >> + struct clk *clk; >> +}; >> + >> +#define to_qcom_cbf_provider(_provider) \ >> + container_of(_provider, struct qcom_msm8996_cbf_icc_provider, provider) >> + >> +enum { >> + CBF_MASTER_NODE = 2000, > [...] >> +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev) >> +{ >> + struct icc_provider *provider = platform_get_drvdata(pdev); >> + >> + icc_nodes_remove(provider); >> + icc_provider_del(provider); >> + >> + return 0; >> +} >> +#else >> +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev) >> +{ >> + dev_warn(&pdev->dev, "interconnects support is disabled, CBF clock is fixed\n"); >> + >> + return 0; >> +} >> +#define qcom_msm8996_cbf_icc_remove(pdev) (0) > > It's like two drivers in one. > >> +#endif >> + >> static int qcom_msm8996_cbf_probe(struct platform_device *pdev) >> { >> void __iomem *base;
Quoting Dmitry Baryshkov (2023-01-20 14:53:21) > On 21/01/2023 00:11, Stephen Boyd wrote: > > Quoting Dmitry Baryshkov (2023-01-19 22:14:13) > >> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c > >> index 9cde0e660228..b049b4f7b270 100644 > >> --- a/drivers/clk/qcom/clk-cbf-8996.c > >> +++ b/drivers/clk/qcom/clk-cbf-8996.c > >> @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { > >> .val_format_endian = REGMAP_ENDIAN_LITTLE, > >> }; > >> > >> +#ifdef CONFIG_INTERCONNECT > > > > Can you move this driver to drivers/interconnect/ ? > > Only the interconnect part? At some point I considered dropping the Yes only the interconnect part. Use auxiliary bus. > whole CBF mux support and moving the whole driver to > drivers/interconnect, but I could not find a good way to use alpha-pll > from the interconnect driver. Would you recommend one? I don't think you need to use alpha-pll code from the interconnect driver, do you?
On 25/01/2023 23:40, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2023-01-20 14:53:21) >> On 21/01/2023 00:11, Stephen Boyd wrote: >>> Quoting Dmitry Baryshkov (2023-01-19 22:14:13) >>>> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c >>>> index 9cde0e660228..b049b4f7b270 100644 >>>> --- a/drivers/clk/qcom/clk-cbf-8996.c >>>> +++ b/drivers/clk/qcom/clk-cbf-8996.c >>>> @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { >>>> .val_format_endian = REGMAP_ENDIAN_LITTLE, >>>> }; >>>> >>>> +#ifdef CONFIG_INTERCONNECT >>> >>> Can you move this driver to drivers/interconnect/ ? >> >> Only the interconnect part? At some point I considered dropping the > > Yes only the interconnect part. Use auxiliary bus. Ack > >> whole CBF mux support and moving the whole driver to >> drivers/interconnect, but I could not find a good way to use alpha-pll >> from the interconnect driver. Would you recommend one? > > I don't think you need to use alpha-pll code from the interconnect > driver, do you? If we have separate drivers? No, I don't.
On 25/01/2023 23:40, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2023-01-20 14:53:21) >> On 21/01/2023 00:11, Stephen Boyd wrote: >>> Quoting Dmitry Baryshkov (2023-01-19 22:14:13) >>>> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c >>>> index 9cde0e660228..b049b4f7b270 100644 >>>> --- a/drivers/clk/qcom/clk-cbf-8996.c >>>> +++ b/drivers/clk/qcom/clk-cbf-8996.c >>>> @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { >>>> .val_format_endian = REGMAP_ENDIAN_LITTLE, >>>> }; >>>> >>>> +#ifdef CONFIG_INTERCONNECT >>> >>> Can you move this driver to drivers/interconnect/ ? >> >> Only the interconnect part? At some point I considered dropping the > > Yes only the interconnect part. Use auxiliary bus. Stephen, Bjorn, since the interconnect parts are already separated (to patches 2, 4, 8, would it be possible to merge the rest into 6.3? Just having the CBF enabled and set to maximum frequency helps to boot msm8996 performance cluster. Without this patchset, it is kind of a lottery, with stable kernel boot achievable only with 'maxcpus=2'.
diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c index 9cde0e660228..b049b4f7b270 100644 --- a/drivers/clk/qcom/clk-cbf-8996.c +++ b/drivers/clk/qcom/clk-cbf-8996.c @@ -5,11 +5,14 @@ #include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/interconnect-provider.h> #include <linux/of.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <dt-bindings/interconnect/qcom,msm8996-cbf.h> + #include "clk-alpha-pll.h" #include "clk-regmap.h" @@ -225,6 +228,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = { .val_format_endian = REGMAP_ENDIAN_LITTLE, }; +#ifdef CONFIG_INTERCONNECT +struct qcom_msm8996_cbf_icc_provider { + struct icc_provider provider; + struct clk *clk; +}; + +#define to_qcom_cbf_provider(_provider) \ + container_of(_provider, struct qcom_msm8996_cbf_icc_provider, provider) + +enum { + CBF_MASTER_NODE = 2000, + CBF_SLAVE_NODE +}; + +#define CBF_NUM_NODES 2 + +static int qcom_msm8996_cbf_set(struct icc_node *src, struct icc_node *dst) +{ + struct qcom_msm8996_cbf_icc_provider *qp; + + qp = to_qcom_cbf_provider(src->provider); + + return clk_set_rate(qp->clk, icc_units_to_bps(dst->peak_bw)); +} + +static int qcom_msm8996_cbf_icc_get_bw(struct icc_node *node, u32 *avg, u32 *peak) +{ + struct qcom_msm8996_cbf_icc_provider *qp; + + qp = to_qcom_cbf_provider(node->provider); + *peak = clk_get_rate(qp->clk) / 1000ULL; + + return 0; +} + +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw) +{ + struct device *dev = &pdev->dev; + struct qcom_msm8996_cbf_icc_provider *qp; + struct icc_provider *provider; + struct icc_onecell_data *data; + struct icc_node *node; + struct clk *clk; + int ret; + + clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + data = devm_kzalloc(dev, struct_size(data, nodes, CBF_NUM_NODES), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->num_nodes = CBF_NUM_NODES; + + qp = devm_kzalloc(dev, sizeof(*qp), GFP_KERNEL); + if (!qp) + return -ENOMEM; + + qp->clk = clk; + + provider = &qp->provider; + provider->dev = dev; + provider->get_bw = qcom_msm8996_cbf_icc_get_bw; + provider->set = qcom_msm8996_cbf_set; + provider->aggregate = icc_std_aggregate; + provider->xlate = of_icc_xlate_onecell; + INIT_LIST_HEAD(&provider->nodes); + provider->data = data; + + ret = icc_provider_add(provider); + if (ret) { + dev_err(dev, "error adding interconnect provider\n"); + return ret; + } + + node = icc_node_create(CBF_MASTER_NODE); + if (IS_ERR(node)) { + ret = PTR_ERR(node); + goto err; + } + + node->name = "cbf_master"; + icc_node_add(node, provider); + icc_link_create(node, CBF_SLAVE_NODE); + data->nodes[MASTER_CBF_M4M] = node; + + node = icc_node_create(CBF_SLAVE_NODE); + if (IS_ERR(node)) { + ret = PTR_ERR(node); + goto err; + } + + node->name = "cbf_slave"; + icc_node_add(node, provider); + data->nodes[SLAVE_CBF_M4M] = node; + + platform_set_drvdata(pdev, provider); + + return 0; + +err: + icc_nodes_remove(provider); + icc_provider_del(provider); + + return ret; +} + +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev) +{ + struct icc_provider *provider = platform_get_drvdata(pdev); + + icc_nodes_remove(provider); + icc_provider_del(provider); + + return 0; +} +#else +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev) +{ + dev_warn(&pdev->dev, "interconnects support is disabled, CBF clock is fixed\n"); + + return 0; +} +#define qcom_msm8996_cbf_icc_remove(pdev) (0) +#endif + static int qcom_msm8996_cbf_probe(struct platform_device *pdev) { void __iomem *base; @@ -283,7 +413,16 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev) if (ret) return ret; - return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw); + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw); + if (ret) + return ret; + + return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw); +} + +static int qcom_msm8996_cbf_remove(struct platform_device *pdev) +{ + return qcom_msm8996_cbf_icc_remove(pdev); } static const struct of_device_id qcom_msm8996_cbf_match_table[] = { @@ -294,9 +433,11 @@ MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table); static struct platform_driver qcom_msm8996_cbf_driver = { .probe = qcom_msm8996_cbf_probe, + .remove = qcom_msm8996_cbf_remove, .driver = { .name = "qcom-msm8996-cbf", .of_match_table = qcom_msm8996_cbf_match_table, + .sync_state = icc_sync_state, }, };
Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth) according to CPU frequencies. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/clk/qcom/clk-cbf-8996.c | 143 +++++++++++++++++++++++++++++++- 1 file changed, 142 insertions(+), 1 deletion(-)