[PATCH v6 2/6] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[PATCH v6 2/6] spi: spi-geni-qcom: Use OPP API to set clk/perf state

[Rajendra Nayak]

geni spi needs to express a perforamnce state requirement on CX
depending on the frequency of the clock rates. Use OPP table from
DT to register with OPP framework and use dev_pm_opp_set_rate() to
set the clk/perf state.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: Mark Brown <broonie@kernel.org>
Cc: Alok Chauhan <alokc@codeaurora.org>
Cc: Akash Asthana <akashast@codeaurora.org>
Cc: linux-spi@vger.kernel.org
---
This patch needs to land via the msm tree. Mark has acked v5, so
this is good to land I think. v6 is just rebased on 5.8-rc1.

 drivers/spi/spi-geni-qcom.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index c397242..0d7ead1 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -7,6 +7,7 @@
 #include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/qcom-geni-se.h>
 #include <linux/spi/spi.h>
@@ -95,7 +96,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
 {
 	unsigned long sclk_freq;
 	unsigned int actual_hz;
-	struct geni_se *se = &mas->se;
 	int ret;
 
 	ret = geni_se_clk_freq_match(&mas->se,
@@ -112,9 +112,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
 
 	dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz,
 				actual_hz, sclk_freq, *clk_idx, *clk_div);
-	ret = clk_set_rate(se->clk, sclk_freq);
+	ret = dev_pm_opp_set_rate(mas->dev, sclk_freq);
 	if (ret)
-		dev_err(mas->dev, "clk_set_rate failed %d\n", ret);
+		dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret);
 	return ret;
 }
 
@@ -561,6 +561,17 @@ static int spi_geni_probe(struct platform_device *pdev)
 	mas->se.wrapper = dev_get_drvdata(dev->parent);
 	mas->se.base = base;
 	mas->se.clk = clk;
+	mas->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se");
+	if (IS_ERR(mas->se.opp_table))
+		return PTR_ERR(mas->se.opp_table);
+	/* OPP table is optional */
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (!ret) {
+		mas->se.has_opp_table = true;
+	} else if (ret != -ENODEV) {
+		dev_err(&pdev->dev, "invalid OPP table in device tree\n");
+		return ret;
+	}
 
 	spi->bus_num = -1;
 	spi->dev.of_node = dev->of_node;
@@ -596,6 +607,9 @@ static int spi_geni_probe(struct platform_device *pdev)
 spi_geni_probe_runtime_disable:
 	pm_runtime_disable(dev);
 	spi_master_put(spi);
+	if (mas->se.has_opp_table)
+		dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(mas->se.opp_table);
 	return ret;
 }
 
@@ -609,6 +623,9 @@ static int spi_geni_remove(struct platform_device *pdev)
 
 	free_irq(mas->irq, spi);
 	pm_runtime_disable(&pdev->dev);
+	if (mas->se.has_opp_table)
+		dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(mas->se.opp_table);
 	return 0;
 }
 
@@ -617,6 +634,9 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
 	struct spi_master *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_master_get_devdata(spi);
 
+	/* Drop the performance state vote */
+	dev_pm_opp_set_rate(dev, 0);
+
 	return geni_se_resources_off(&mas->se);
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Rajendra Nayak]

QSPI needs to vote on a performance state of a power domain depending on
the clock rate. Add support for it by specifying the perf state/clock rate
as an OPP table in device tree.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: Mark Brown <broonie@kernel.org>
Cc: Alok Chauhan <alokc@codeaurora.org>
Cc: Akash Asthana <akashast@codeaurora.org>
Cc: linux-spi@vger.kernel.org
---
No functional change in v6, rebased over 5.8-rc1

 drivers/spi/spi-qcom-qspi.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 3c4f83b..ef51982 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -8,6 +8,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_opp.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 
@@ -139,6 +140,8 @@ struct qcom_qspi {
 	struct device *dev;
 	struct clk_bulk_data *clks;
 	struct qspi_xfer xfer;
+	struct opp_table *opp_table;
+	bool has_opp_table;
 	/* Lock to protect xfer and IRQ accessed registers */
 	spinlock_t lock;
 };
@@ -235,7 +238,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master,
 		speed_hz = xfer->speed_hz;
 
 	/* In regular operation (SBL_EN=1) core must be 4x transfer clock */
-	ret = clk_set_rate(ctrl->clks[QSPI_CLK_CORE].clk, speed_hz * 4);
+	ret = dev_pm_opp_set_rate(ctrl->dev, speed_hz * 4);
 	if (ret) {
 		dev_err(ctrl->dev, "Failed to set core clk %d\n", ret);
 		return ret;
@@ -481,6 +484,20 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	master->handle_err = qcom_qspi_handle_err;
 	master->auto_runtime_pm = true;
 
+	ctrl->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
+	if (IS_ERR(ctrl->opp_table)) {
+		ret = PTR_ERR(ctrl->opp_table);
+		goto exit_probe_master_put;
+	}
+	/* OPP table is optional */
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (!ret) {
+		ctrl->has_opp_table = true;
+	} else if (ret != -ENODEV) {
+		dev_err(&pdev->dev, "invalid OPP table in device tree\n");
+		goto exit_probe_master_put;
+	}
+
 	pm_runtime_enable(dev);
 
 	ret = spi_register_master(master);
@@ -488,6 +505,9 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 		return 0;
 
 	pm_runtime_disable(dev);
+	if (ctrl->has_opp_table)
+		dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(ctrl->opp_table);
 
 exit_probe_master_put:
 	spi_master_put(master);
@@ -498,11 +518,15 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 static int qcom_qspi_remove(struct platform_device *pdev)
 {
 	struct spi_master *master = platform_get_drvdata(pdev);
+	struct qcom_qspi *ctrl = spi_master_get_devdata(master);
 
 	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
 	spi_unregister_master(master);
 
 	pm_runtime_disable(&pdev->dev);
+	if (ctrl->has_opp_table)
+		dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(ctrl->opp_table);
 
 	return 0;
 }
@@ -512,6 +536,8 @@ 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);
 
+	/* Drop the performance state vote */
+	dev_pm_opp_set_rate(dev, 0);
 	clk_bulk_disable_unprepare(QSPI_NUM_CLKS, ctrl->clks);
 
 	return 0;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

Hi Mark,

do you plan to land this in your tree?

I know you hate contentless pings, but since you acked this patch and
usually don't seem to do that when patches go through your tree I want
to make sure we aren't in a situation where everybody thinks that the
patch will go through someone else's tree.

Thanks

Matthias

On Mon, Jun 15, 2020 at 05:32:44PM +0530, Rajendra Nayak wrote:
> QSPI needs to vote on a performance state of a power domain depending on
> the clock rate. Add support for it by specifying the perf state/clock rate
> as an OPP table in device tree.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Acked-by: Mark Brown <broonie@kernel.org>
> Cc: Alok Chauhan <alokc@codeaurora.org>
> Cc: Akash Asthana <akashast@codeaurora.org>
> Cc: linux-spi@vger.kernel.org
> ---
> No functional change in v6, rebased over 5.8-rc1
> 
>  drivers/spi/spi-qcom-qspi.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
> index 3c4f83b..ef51982 100644
> --- a/drivers/spi/spi-qcom-qspi.c
> +++ b/drivers/spi/spi-qcom-qspi.c
> @@ -8,6 +8,7 @@
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pm_opp.h>
>  #include <linux/spi/spi.h>
>  #include <linux/spi/spi-mem.h>
>  
> @@ -139,6 +140,8 @@ struct qcom_qspi {
>  	struct device *dev;
>  	struct clk_bulk_data *clks;
>  	struct qspi_xfer xfer;
> +	struct opp_table *opp_table;
> +	bool has_opp_table;
>  	/* Lock to protect xfer and IRQ accessed registers */
>  	spinlock_t lock;
>  };
> @@ -235,7 +238,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master,
>  		speed_hz = xfer->speed_hz;
>  
>  	/* In regular operation (SBL_EN=1) core must be 4x transfer clock */
> -	ret = clk_set_rate(ctrl->clks[QSPI_CLK_CORE].clk, speed_hz * 4);
> +	ret = dev_pm_opp_set_rate(ctrl->dev, speed_hz * 4);
>  	if (ret) {
>  		dev_err(ctrl->dev, "Failed to set core clk %d\n", ret);
>  		return ret;
> @@ -481,6 +484,20 @@ static int qcom_qspi_probe(struct platform_device *pdev)
>  	master->handle_err = qcom_qspi_handle_err;
>  	master->auto_runtime_pm = true;
>  
> +	ctrl->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
> +	if (IS_ERR(ctrl->opp_table)) {
> +		ret = PTR_ERR(ctrl->opp_table);
> +		goto exit_probe_master_put;
> +	}
> +	/* OPP table is optional */
> +	ret = dev_pm_opp_of_add_table(&pdev->dev);
> +	if (!ret) {
> +		ctrl->has_opp_table = true;
> +	} else if (ret != -ENODEV) {
> +		dev_err(&pdev->dev, "invalid OPP table in device tree\n");
> +		goto exit_probe_master_put;
> +	}
> +
>  	pm_runtime_enable(dev);
>  
>  	ret = spi_register_master(master);
> @@ -488,6 +505,9 @@ static int qcom_qspi_probe(struct platform_device *pdev)
>  		return 0;
>  
>  	pm_runtime_disable(dev);
> +	if (ctrl->has_opp_table)
> +		dev_pm_opp_of_remove_table(&pdev->dev);
> +	dev_pm_opp_put_clkname(ctrl->opp_table);
>  
>  exit_probe_master_put:
>  	spi_master_put(master);
> @@ -498,11 +518,15 @@ static int qcom_qspi_probe(struct platform_device *pdev)
>  static int qcom_qspi_remove(struct platform_device *pdev)
>  {
>  	struct spi_master *master = platform_get_drvdata(pdev);
> +	struct qcom_qspi *ctrl = spi_master_get_devdata(master);
>  
>  	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
>  	spi_unregister_master(master);
>  
>  	pm_runtime_disable(&pdev->dev);
> +	if (ctrl->has_opp_table)
> +		dev_pm_opp_of_remove_table(&pdev->dev);
> +	dev_pm_opp_put_clkname(ctrl->opp_table);
>  
>  	return 0;
>  }
> @@ -512,6 +536,8 @@ 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);
>  
> +	/* Drop the performance state vote */
> +	dev_pm_opp_set_rate(dev, 0);
>  	clk_bulk_disable_unprepare(QSPI_NUM_CLKS, ctrl->clks);
>  
>  	return 0;
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Wed, Jun 24, 2020 at 10:09:33AM -0700, Matthias Kaehlcke wrote:
> Hi Mark,
> 
> do you plan to land this in your tree?
> 
> I know you hate contentless pings, but since you acked this patch and
> usually don't seem to do that when patches go through your tree I want
> to make sure we aren't in a situation where everybody thinks that the
> patch will go through someone else's tree.

Aren't there dependencies on earlier patches in the series?  In general
if someone acks something for their tree that means they don't expect to
apply it themselves.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

On Wed, Jun 24, 2020 at 06:15:37PM +0100, Mark Brown wrote:
> On Wed, Jun 24, 2020 at 10:09:33AM -0700, Matthias Kaehlcke wrote:
> > Hi Mark,
> > 
> > do you plan to land this in your tree?
> > 
> > I know you hate contentless pings, but since you acked this patch and
> > usually don't seem to do that when patches go through your tree I want
> > to make sure we aren't in a situation where everybody thinks that the
> > patch will go through someone else's tree.
> 
> Aren't there dependencies on earlier patches in the series?

Not to my knowledge. Patch "[2/6] spi: spi-geni-qcom: Use OPP API to set
clk/perf state" depends on a change in 'include/linux/qcom-geni-se.h' made
by "1/6] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state",
however that's not true for this patch.

I wonder if it would have been better to split this series into individual
patches/mini-series, to avoid this kind of confusion.

> In general if someone acks something for their tree that means they don't
> expect to apply it themselves.

Yes, that was my understanding and prompted me to clarify this with you.

The patch could go through the QCOM tree, but to my knowledge there is no
reason for it.

Btw, the patch "[V8,7/8] spi: spi-qcom-qspi: Add interconnect support"
(https://patchwork.kernel.org/patch/11620285/) is in a similar situation.
Another patch of the series for the 'spi-geni-qcom' driver has to go
through the QCOM change due to changes in geni, but the QSPI driver
doesn't use geni and could therefore go through your tree.

Ultimately I don't really care too much through which tree the patches
land as long as you and Bjorn agree on it :)

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Wed, Jun 24, 2020 at 10:39:48AM -0700, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 06:15:37PM +0100, Mark Brown wrote:

> > Aren't there dependencies on earlier patches in the series?

> Not to my knowledge. Patch "[2/6] spi: spi-geni-qcom: Use OPP API to set
> clk/perf state" depends on a change in 'include/linux/qcom-geni-se.h' made
> by "1/6] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state",
> however that's not true for this patch.

Wait, so *some* of the series should go together but not other bits?
But you want them split up for some reason?

> I wonder if it would have been better to split this series into individual
> patches/mini-series, to avoid this kind of confusion.

Yes, if there's no dependencies then bundling things up into a series
just causes confusion.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:
> On Wed, Jun 24, 2020 at 10:39:48AM -0700, Matthias Kaehlcke wrote:
> > On Wed, Jun 24, 2020 at 06:15:37PM +0100, Mark Brown wrote:
> 
> > > Aren't there dependencies on earlier patches in the series?
> 
> > Not to my knowledge. Patch "[2/6] spi: spi-geni-qcom: Use OPP API to set
> > clk/perf state" depends on a change in 'include/linux/qcom-geni-se.h' made
> > by "1/6] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state",
> > however that's not true for this patch.
> 
> Wait, so *some* of the series should go together but not other bits?
> But you want them split up for some reason?

Yes, this will almost certainly be the case, even if not for this patch.
I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).

It seems very unlikely to me that the DRM patches will go through the QCOM
tree. The venus patch also doesn't have any dependencies and is more likely
to cause conflicts if it lands through QCOM instead of it's maintainer tree.
For the QSPI patch you could argue to just take it through QCOM since the SPI
patch of this series goes through this tree, up to you, I just want to make
sure everybody is on the same page.

> > I wonder if it would have been better to split this series into individual
> > patches/mini-series, to avoid this kind of confusion.
> 
> Yes, if there's no dependencies then bundling things up into a series
> just causes confusion.

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Wed, Jun 24, 2020 at 10:55:36AM -0700, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:

> > Wait, so *some* of the series should go together but not other bits?
> > But you want them split up for some reason?

> Yes, this will almost certainly be the case, even if not for this patch.
> I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).

I'm not really reading any of this stuff for the series as a whole, as
far as I could tell I'd reviewed all my bits and was hoping whatever
random platform stuff needs sorting out was going to be sorted out so I
stopped getting copied on revisions :(

> For the QSPI patch you could argue to just take it through QCOM since the SPI
> patch of this series goes through this tree, up to you, I just want to make
> sure everybody is on the same page.

If there are some part of this that don't have a connection with the
rest of the series and should be applied separately please split them
out and send them separately so it's clear what's going on.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

On Wed, Jun 24, 2020 at 07:00:05PM +0100, Mark Brown wrote:
> On Wed, Jun 24, 2020 at 10:55:36AM -0700, Matthias Kaehlcke wrote:
> > On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:
> 
> > > Wait, so *some* of the series should go together but not other bits?
> > > But you want them split up for some reason?
> 
> > Yes, this will almost certainly be the case, even if not for this patch.
> > I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).
> 
> I'm not really reading any of this stuff for the series as a whole, as
> far as I could tell I'd reviewed all my bits and was hoping whatever
> random platform stuff needs sorting out was going to be sorted out so I
> stopped getting copied on revisions :(

Sorry this caused you extra work, I only fully realized this when the series
was basically ready to land :(

Avoiding unnecessary revision spam is another good reason to not combine
technically unrelated patches in a single series.

If I notice similar series in the future I'll try to bring it up early.

> > For the QSPI patch you could argue to just take it through QCOM since the SPI
> > patch of this series goes through this tree, up to you, I just want to make
> > sure everybody is on the same page.
> 
> If there are some part of this that don't have a connection with the
> rest of the series and should be applied separately please split them
> out and send them separately so it's clear what's going on.

Rajendra, IIUC you have to re-spin this series anyway, please split it
up in self-contained chunks.

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Wed, Jun 24, 2020 at 11:12:45AM -0700, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 07:00:05PM +0100, Mark Brown wrote:

> > I'm not really reading any of this stuff for the series as a whole, as
> > far as I could tell I'd reviewed all my bits and was hoping whatever
> > random platform stuff needs sorting out was going to be sorted out so I
> > stopped getting copied on revisions :(

> Sorry this caused you extra work, I only fully realized this when the series
> was basically ready to land :(

It's fine, mostly it's just checking to see that I'd reviewed all the
relevant patches which takes moments.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Matthias Kaehlcke]

On Wed, Jun 24, 2020 at 11:12:45AM -0700, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 07:00:05PM +0100, Mark Brown wrote:
> > On Wed, Jun 24, 2020 at 10:55:36AM -0700, Matthias Kaehlcke wrote:
> > > On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:
> > 
> > > > Wait, so *some* of the series should go together but not other bits?
> > > > But you want them split up for some reason?
> > 
> > > Yes, this will almost certainly be the case, even if not for this patch.
> > > I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).
> > 
> > I'm not really reading any of this stuff for the series as a whole, as
> > far as I could tell I'd reviewed all my bits and was hoping whatever
> > random platform stuff needs sorting out was going to be sorted out so I
> > stopped getting copied on revisions :(
> 
> Sorry this caused you extra work, I only fully realized this when the series
> was basically ready to land :(
> 
> Avoiding unnecessary revision spam is another good reason to not combine
> technically unrelated patches in a single series.
> 
> If I notice similar series in the future I'll try to bring it up early.
> 
> > > For the QSPI patch you could argue to just take it through QCOM since the SPI
> > > patch of this series goes through this tree, up to you, I just want to make
> > > sure everybody is on the same page.
> > 
> > If there are some part of this that don't have a connection with the
> > rest of the series and should be applied separately please split them
> > out and send them separately so it's clear what's going on.
> 
> Rajendra, IIUC you have to re-spin this series anyway, please split it
> up in self-contained chunks.

One more thing: when you do the split it seems it would make sense to
include the DT changes that were initially part of this series
(https://patchwork.kernel.org/project/linux-arm-msm/list/?series=278691&state=*)

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Rajendra Nayak]

On 6/25/2020 8:55 PM, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 11:12:45AM -0700, Matthias Kaehlcke wrote:
>> On Wed, Jun 24, 2020 at 07:00:05PM +0100, Mark Brown wrote:
>>> On Wed, Jun 24, 2020 at 10:55:36AM -0700, Matthias Kaehlcke wrote:
>>>> On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:
>>>
>>>>> Wait, so *some* of the series should go together but not other bits?
>>>>> But you want them split up for some reason?
>>>
>>>> Yes, this will almost certainly be the case, even if not for this patch.
>>>> I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).
>>>
>>> I'm not really reading any of this stuff for the series as a whole, as
>>> far as I could tell I'd reviewed all my bits and was hoping whatever
>>> random platform stuff needs sorting out was going to be sorted out so I
>>> stopped getting copied on revisions :(
>>
>> Sorry this caused you extra work, I only fully realized this when the series
>> was basically ready to land :(
>>
>> Avoiding unnecessary revision spam is another good reason to not combine
>> technically unrelated patches in a single series.
>>
>> If I notice similar series in the future I'll try to bring it up early.
>>
>>>> For the QSPI patch you could argue to just take it through QCOM since the SPI
>>>> patch of this series goes through this tree, up to you, I just want to make
>>>> sure everybody is on the same page.
>>>
>>> If there are some part of this that don't have a connection with the
>>> rest of the series and should be applied separately please split them
>>> out and send them separately so it's clear what's going on.
>>
>> Rajendra, IIUC you have to re-spin this series anyway, please split it
>> up in self-contained chunks.
> 
> One more thing: when you do the split it seems it would make sense to
> include the DT changes that were initially part of this series
> (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=278691&state=*)

Sure, I'll send the ones out for which driver changes are already merged/pulled in,
like sdhc, geni-uart and geni-spi.
For the rest, I will include them with the driver changes.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v6 6/6] spi: spi-qcom-qspi: Use OPP API to set clk/perf state

[Rajendra Nayak]

On 6/24/2020 11:42 PM, Matthias Kaehlcke wrote:
> On Wed, Jun 24, 2020 at 07:00:05PM +0100, Mark Brown wrote:
>> On Wed, Jun 24, 2020 at 10:55:36AM -0700, Matthias Kaehlcke wrote:
>>> On Wed, Jun 24, 2020 at 06:44:17PM +0100, Mark Brown wrote:
>>
>>>> Wait, so *some* of the series should go together but not other bits?
>>>> But you want them split up for some reason?
>>
>>> Yes, this will almost certainly be the case, even if not for this patch.
>>> I brought this up earlier (https://patchwork.kernel.org/cover/11604623/#23428709).
>>
>> I'm not really reading any of this stuff for the series as a whole, as
>> far as I could tell I'd reviewed all my bits and was hoping whatever
>> random platform stuff needs sorting out was going to be sorted out so I
>> stopped getting copied on revisions :(
> 
> Sorry this caused you extra work, I only fully realized this when the series
> was basically ready to land :(
> 
> Avoiding unnecessary revision spam is another good reason to not combine
> technically unrelated patches in a single series.
> 
> If I notice similar series in the future I'll try to bring it up early.
> 
>>> For the QSPI patch you could argue to just take it through QCOM since the SPI
>>> patch of this series goes through this tree, up to you, I just want to make
>>> sure everybody is on the same page.
>>
>> If there are some part of this that don't have a connection with the
>> rest of the series and should be applied separately please split them
>> out and send them separately so it's clear what's going on.
> 
> Rajendra, IIUC you have to re-spin this series anyway, please split it
> up in self-contained chunks.

Thanks, I'll respin these as separate patches, the only reason to club them
was because they all added 'similar' support in all these different drivers.
Sorry for the delay, I had been out a bit and I just got back.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation