diff mbox series

[V7,RESEND,6/7] spi: spi-qcom-qspi: Add interconnect support

Message ID 1591682194-32388-7-git-send-email-akashast@codeaurora.org
State Superseded
Headers show
Series [V7,RESEND,1/7] soc: qcom: geni: Support for ICC voting | expand

Commit Message

Akash Asthana June 9, 2020, 5:56 a.m. UTC
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(-)

Comments

Akash Asthana June 15, 2020, 7:27 a.m. UTC | #1
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);
>   }
Mark Brown June 15, 2020, 12:01 p.m. UTC | #2
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 mbox series

Patch

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);
 }