| Message ID | 1591682194-32388-7-git-send-email-akashast@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add interconnect support to QSPI and QUP drivers | expand |
Hi Mark, Would you be able to review/ack this QSPI patch, you have already acked "QUP SPI" patch from the series "[Patch V7 RESEND 4/7]" Putting a gentle reminder in-case this patch is missed. Regards, Akash On 6/9/2020 11:26 AM, 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> > Reviewed-by: Matthias Kaehlcke <mka@chromium.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. > > Changes in Resend V7: > - As per Matthias comment removed "unsigned int avg_bw_cpu" from > struct qcom_qspi as we are using that variable only once. > > drivers/spi/spi-qcom-qspi.c | 56 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 55 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c > index 3c4f83b..b5b4cf6 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,8 @@ 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; > + /* Lock to protect data accessed by IRQs */ > spinlock_t lock; > }; > > @@ -229,6 +231,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master, > int ret; > unsigned long speed_hz; > unsigned long flags; > + unsigned int avg_bw_cpu; > > speed_hz = slv->max_speed_hz; > if (xfer->speed_hz) > @@ -241,6 +244,18 @@ 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. > + */ > + avg_bw_cpu = Bps_to_icc(speed_hz); > + ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, avg_bw_cpu, 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 +473,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 +549,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 +567,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); > }
On Mon, Jun 15, 2020 at 12:57:12PM +0530, Akash Asthana wrote: > Hi Mark, > > Would you be able to review/ack this QSPI patch, you have already acked "QUP > SPI" patch from the series "[Patch V7 RESEND 4/7]" > > Putting a gentle reminder in-case this patch is missed. Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. If there have been review comments then people may be waiting for those to be addressed. Sending content free pings adds to the mail volume (if they are seen at all) which is often the problem and since they can't be reviewed directly if something has gone wrong you'll have to resend the patches anyway, so sending again is generally a better approach though there are some other maintainers who like them - if in doubt look at how patches for the subsystem are normally handled.
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 3c4f83b..b5b4cf6 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,8 @@ 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; + /* Lock to protect data accessed by IRQs */ spinlock_t lock; }; @@ -229,6 +231,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master, int ret; unsigned long speed_hz; unsigned long flags; + unsigned int avg_bw_cpu; speed_hz = slv->max_speed_hz; if (xfer->speed_hz) @@ -241,6 +244,18 @@ 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. + */ + avg_bw_cpu = Bps_to_icc(speed_hz); + ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, avg_bw_cpu, 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 +473,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 +549,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 +567,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); }