| Message ID | 1590497690-29035-7-git-send-email-akashast@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add interconnect support to QSPI and QUP drivers | expand |
On Tue, May 26, 2020 at 06:24:49PM +0530, Akash Asthana wrote: > Get the interconnect paths for QSPI device and vote according to the > current bus speed of the driver. > > Signed-off-by: Akash Asthana <akashast@codeaurora.org> > --- > Changes in V2: > - As per Bjorn's comment, introduced and using devm_of_icc_get API for getting > path handle > - As per Matthias comment, added error handling for icc_set_bw call > > Changes in V3: > - No Change. > > Changes in V4: > - As per Mark's comment move peak_bw guess as twice of avg_bw if > nothing mentioned explicitly to ICC core. > > Changes in V5: > - Add icc_enable/disable to power on/off call. > - Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw > from probe so that when resume/icc_enable is called NOC are running at > some non-zero value. > > Changes in V6: > - As per Matthias's comment made print statement consistent across driver > > Changes in V7: > - As per Matthias's comment removed usage of peak_bw variable because we don't > have explicit peak requirement, we were voting peak = avg and this can be > tracked using single variable for avg bw. > - As per Matthias's comment improved print log. > > drivers/spi/spi-qcom-qspi.c | 57 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 56 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c > index 3c4f83b..092ac27 100644 > --- a/drivers/spi/spi-qcom-qspi.c > +++ b/drivers/spi/spi-qcom-qspi.c > @@ -2,6 +2,7 @@ > // Copyright (c) 2017-2018, The Linux foundation. All rights reserved. > > #include <linux/clk.h> > +#include <linux/interconnect.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/module.h> > @@ -139,7 +140,9 @@ struct qcom_qspi { > struct device *dev; > struct clk_bulk_data *clks; > struct qspi_xfer xfer; > - /* Lock to protect xfer and IRQ accessed registers */ > + struct icc_path *icc_path_cpu_to_qspi; > + unsigned int avg_bw_cpu; I should have noticed this earlier, but the field isn't needed now that we have icc_enable/disable(). The bandwidth is set in qcom_qspi_transfer_one() and that's it. From my side it would be fine to remove the field in a follow up patch, to avoid respinning the series yet another time just for this. Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Hi Matthias, On 5/26/2020 11:06 PM, Matthias Kaehlcke wrote: > On Tue, May 26, 2020 at 06:24:49PM +0530, Akash Asthana wrote: >> Get the interconnect paths for QSPI device and vote according to the >> current bus speed of the driver. >> >> Signed-off-by: Akash Asthana <akashast@codeaurora.org> >> --- >> Changes in V2: >> - As per Bjorn's comment, introduced and using devm_of_icc_get API for getting >> path handle >> - As per Matthias comment, added error handling for icc_set_bw call >> >> Changes in V3: >> - No Change. >> >> Changes in V4: >> - As per Mark's comment move peak_bw guess as twice of avg_bw if >> nothing mentioned explicitly to ICC core. >> >> Changes in V5: >> - Add icc_enable/disable to power on/off call. >> - Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw >> from probe so that when resume/icc_enable is called NOC are running at >> some non-zero value. >> >> Changes in V6: >> - As per Matthias's comment made print statement consistent across driver >> >> Changes in V7: >> - As per Matthias's comment removed usage of peak_bw variable because we don't >> have explicit peak requirement, we were voting peak = avg and this can be >> tracked using single variable for avg bw. >> - As per Matthias's comment improved print log. >> >> drivers/spi/spi-qcom-qspi.c | 57 ++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 56 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c >> index 3c4f83b..092ac27 100644 >> --- a/drivers/spi/spi-qcom-qspi.c >> +++ b/drivers/spi/spi-qcom-qspi.c >> @@ -2,6 +2,7 @@ >> // Copyright (c) 2017-2018, The Linux foundation. All rights reserved. >> >> #include <linux/clk.h> >> +#include <linux/interconnect.h> >> #include <linux/interrupt.h> >> #include <linux/io.h> >> #include <linux/module.h> >> @@ -139,7 +140,9 @@ struct qcom_qspi { >> struct device *dev; >> struct clk_bulk_data *clks; >> struct qspi_xfer xfer; >> - /* Lock to protect xfer and IRQ accessed registers */ >> + struct icc_path *icc_path_cpu_to_qspi; >> + unsigned int avg_bw_cpu; > I should have noticed this earlier, but the field isn't needed now that > we have icc_enable/disable(). The bandwidth is set in > qcom_qspi_transfer_one() and that's it. > > From my side it would be fine to remove the field in a follow up patch, > to avoid respinning the series yet another time just for this. > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> I will resend patch V7 with the suggested change. Thankyou for reviewing Regards, Akash
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 3c4f83b..092ac27 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -2,6 +2,7 @@ // Copyright (c) 2017-2018, The Linux foundation. All rights reserved. #include <linux/clk.h> +#include <linux/interconnect.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h> @@ -139,7 +140,9 @@ struct qcom_qspi { struct device *dev; struct clk_bulk_data *clks; struct qspi_xfer xfer; - /* Lock to protect xfer and IRQ accessed registers */ + struct icc_path *icc_path_cpu_to_qspi; + unsigned int avg_bw_cpu; + /* Lock to protect data accessed by IRQs */ spinlock_t lock; }; @@ -241,6 +244,19 @@ static int qcom_qspi_transfer_one(struct spi_master *master, return ret; } + /* + * Set BW quota for CPU as driver supports FIFO mode only. + * We don't have explicit peak requirement so keep it equal to avg_bw. + */ + ctrl->avg_bw_cpu = Bps_to_icc(speed_hz); + ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, ctrl->avg_bw_cpu, + ctrl->avg_bw_cpu); + if (ret) { + dev_err(ctrl->dev, "%s: ICC BW voting failed for cpu: %d\n", + __func__, ret); + return ret; + } + spin_lock_irqsave(&ctrl->lock, flags); /* We are half duplex, so either rx or tx will be set */ @@ -458,6 +474,29 @@ static int qcom_qspi_probe(struct platform_device *pdev) if (ret) goto exit_probe_master_put; + ctrl->icc_path_cpu_to_qspi = devm_of_icc_get(dev, "qspi-config"); + if (IS_ERR(ctrl->icc_path_cpu_to_qspi)) { + ret = PTR_ERR(ctrl->icc_path_cpu_to_qspi); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get cpu path: %d\n", ret); + goto exit_probe_master_put; + } + /* Set BW vote for register access */ + ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, Bps_to_icc(1000), + Bps_to_icc(1000)); + if (ret) { + dev_err(ctrl->dev, "%s: ICC BW voting failed for cpu: %d\n", + __func__, ret); + goto exit_probe_master_put; + } + + ret = icc_disable(ctrl->icc_path_cpu_to_qspi); + if (ret) { + dev_err(ctrl->dev, "%s: ICC disable failed for cpu: %d\n", + __func__, ret); + goto exit_probe_master_put; + } + ret = platform_get_irq(pdev, 0); if (ret < 0) goto exit_probe_master_put; @@ -511,9 +550,17 @@ static int __maybe_unused qcom_qspi_runtime_suspend(struct device *dev) { struct spi_master *master = dev_get_drvdata(dev); struct qcom_qspi *ctrl = spi_master_get_devdata(master); + int ret; clk_bulk_disable_unprepare(QSPI_NUM_CLKS, ctrl->clks); + ret = icc_disable(ctrl->icc_path_cpu_to_qspi); + if (ret) { + dev_err_ratelimited(ctrl->dev, "%s: ICC disable failed for cpu: %d\n", + __func__, ret); + return ret; + } + return 0; } @@ -521,6 +568,14 @@ static int __maybe_unused qcom_qspi_runtime_resume(struct device *dev) { struct spi_master *master = dev_get_drvdata(dev); struct qcom_qspi *ctrl = spi_master_get_devdata(master); + int ret; + + ret = icc_enable(ctrl->icc_path_cpu_to_qspi); + if (ret) { + dev_err_ratelimited(ctrl->dev, "%s: ICC enable failed for cpu: %d\n", + __func__, ret); + return ret; + } return clk_bulk_prepare_enable(QSPI_NUM_CLKS, ctrl->clks); }
Get the interconnect paths for QSPI device and vote according to the current bus speed of the driver. Signed-off-by: Akash Asthana <akashast@codeaurora.org> --- Changes in V2: - As per Bjorn's comment, introduced and using devm_of_icc_get API for getting path handle - As per Matthias comment, added error handling for icc_set_bw call Changes in V3: - No Change. Changes in V4: - As per Mark's comment move peak_bw guess as twice of avg_bw if nothing mentioned explicitly to ICC core. Changes in V5: - Add icc_enable/disable to power on/off call. - Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw from probe so that when resume/icc_enable is called NOC are running at some non-zero value. Changes in V6: - As per Matthias's comment made print statement consistent across driver Changes in V7: - As per Matthias's comment removed usage of peak_bw variable because we don't have explicit peak requirement, we were voting peak = avg and this can be tracked using single variable for avg bw. - As per Matthias's comment improved print log. drivers/spi/spi-qcom-qspi.c | 57 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-)