[PATCH v3 0/5] K2G: Add QSPI support

[PATCH v3 0/5] K2G: Add QSPI support

[Vignesh R]

This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC.
The patches enhance the existing cadence-quadspi driver to support
loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC.

Change log:

v3:
* Fix build warnings reported by kbuild test bot.

Resend:
* Rebase to latest linux-next.
* Collect Acked-bys

v2:
* Drop DT patches. Will be sent as separate series as requested by
 maintainer.
* Split binding docs into separate patches.
* Address comments by Rob Herring.


Vignesh R (5):
  mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible
  mtd: spi-nor: cadence-quadspi: add a delay in write sequence
  mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back
    circuit
  mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock
    circuit
  mtd: spi-nor: cadence-quadspi: Add runtime PM support

 .../devicetree/bindings/mtd/cadence-quadspi.txt    |  7 +++-
 drivers/mtd/spi-nor/cadence-quadspi.c              | 46 ++++++++++++++++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.14.1

[PATCH v3 0/5] K2G: Add QSPI support

[Vignesh R]

This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC.
The patches enhance the existing cadence-quadspi driver to support
loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC.

Change log:

v3:
* Fix build warnings reported by kbuild test bot.

Resend:
* Rebase to latest linux-next.
* Collect Acked-bys

v2:
* Drop DT patches. Will be sent as separate series as requested by
 maintainer.
* Split binding docs into separate patches.
* Address comments by Rob Herring.


Vignesh R (5):
  mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible
  mtd: spi-nor: cadence-quadspi: add a delay in write sequence
  mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back
    circuit
  mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock
    circuit
  mtd: spi-nor: cadence-quadspi: Add runtime PM support

 .../devicetree/bindings/mtd/cadence-quadspi.txt    |  7 +++-
 drivers/mtd/spi-nor/cadence-quadspi.c              | 46 ++++++++++++++++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.14.1

[PATCH v3 0/5] K2G: Add QSPI support

[Vignesh R]

This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC.
The patches enhance the existing cadence-quadspi driver to support
loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC.

Change log:

v3:
* Fix build warnings reported by kbuild test bot.

Resend:
* Rebase to latest linux-next.
* Collect Acked-bys

v2:
* Drop DT patches. Will be sent as separate series as requested by
 maintainer.
* Split binding docs into separate patches.
* Address comments by Rob Herring.


Vignesh R (5):
  mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible
  mtd: spi-nor: cadence-quadspi: add a delay in write sequence
  mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back
    circuit
  mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock
    circuit
  mtd: spi-nor: cadence-quadspi: Add runtime PM support

 .../devicetree/bindings/mtd/cadence-quadspi.txt    |  7 +++-
 drivers/mtd/spi-nor/cadence-quadspi.c              | 46 ++++++++++++++++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.14.1

[PATCH v3 1/5] mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible

[Vignesh R]

Update binding documentation to add a new compatible for TI 66AK2G SoC,
to handle TI SoC specific quirks in the driver.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056da24c..7dbe3bd9ac56 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : should be one of the following:
+	Generic default - "cdns,qspi-nor".
+	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
-- 
2.14.1

[PATCH v3 1/5] mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible

[Vignesh R]

Update binding documentation to add a new compatible for TI 66AK2G SoC,
to handle TI SoC specific quirks in the driver.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056da24c..7dbe3bd9ac56 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : should be one of the following:
+	Generic default - "cdns,qspi-nor".
+	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
-- 
2.14.1

[PATCH v3 1/5] mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible

[Vignesh R]

Update binding documentation to add a new compatible for TI 66AK2G SoC,
to handle TI SoC specific quirks in the driver.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056da24c..7dbe3bd9ac56 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : should be one of the following:
+	Generic default - "cdns,qspi-nor".
+	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
-- 
2.14.1

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
Controller programming sequence, a delay equal to couple of QSPI master
clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
to handle this and set this flag for TI 66AK2G SoC.

[1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v3:
Fix build warnings reported by kbuild test bot.

 drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..5cd5d6f7303f 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -38,6 +38,9 @@
 #define CQSPI_NAME			"cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT		16
 
+/* Quirks */
+#define CQSPI_NEEDS_WR_DELAY		BIT(0)
+
 struct cqspi_st;
 
 struct cqspi_flash_pdata {
@@ -76,6 +79,7 @@ struct cqspi_st {
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			trigger_address;
+	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
 	reinit_completion(&cqspi->transfer_complete);
 	writel(CQSPI_REG_INDIRECTWR_START_MASK,
 	       reg_base + CQSPI_REG_INDIRECTWR);
+	/*
+	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
+	 * Controller programming sequence, couple of cycles of
+	 * QSPI_REF_CLK delay is required for the above bit to
+	 * be internally synchronized by the QSPI module. Provide 5
+	 * cycles of delay.
+	 */
+	if (cqspi->wr_delay)
+		ndelay(cqspi->wr_delay);
 
 	while (remaining > 0) {
 		write_bytes = remaining > page_size ? page_size : remaining;
@@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	struct cqspi_st *cqspi;
 	struct resource *res;
 	struct resource *res_ahb;
+	unsigned long data;
 	int ret;
 	int irq;
 
@@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
+	data  = (unsigned long)of_device_get_match_data(dev);
+	if (data & CQSPI_NEEDS_WR_DELAY)
+		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+						   cqspi->master_ref_clk_hz);
 
 	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
 			       pdev->name, cqspi);
@@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #endif
 
 static const struct of_device_id cqspi_dt_ids[] = {
-	{.compatible = "cdns,qspi-nor",},
+	{
+		.compatible = "cdns,qspi-nor",
+		.data = (void *)0,
+	},
+	{
+		.compatible = "ti,k2g-qspi",
+		.data = (void *)CQSPI_NEEDS_WR_DELAY,
+	},
 	{ /* end of table */ }
 };
 
-- 
2.14.1

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
Controller programming sequence, a delay equal to couple of QSPI master
clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
to handle this and set this flag for TI 66AK2G SoC.

[1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v3:
Fix build warnings reported by kbuild test bot.

 drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..5cd5d6f7303f 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -38,6 +38,9 @@
 #define CQSPI_NAME			"cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT		16
 
+/* Quirks */
+#define CQSPI_NEEDS_WR_DELAY		BIT(0)
+
 struct cqspi_st;
 
 struct cqspi_flash_pdata {
@@ -76,6 +79,7 @@ struct cqspi_st {
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			trigger_address;
+	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
 	reinit_completion(&cqspi->transfer_complete);
 	writel(CQSPI_REG_INDIRECTWR_START_MASK,
 	       reg_base + CQSPI_REG_INDIRECTWR);
+	/*
+	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
+	 * Controller programming sequence, couple of cycles of
+	 * QSPI_REF_CLK delay is required for the above bit to
+	 * be internally synchronized by the QSPI module. Provide 5
+	 * cycles of delay.
+	 */
+	if (cqspi->wr_delay)
+		ndelay(cqspi->wr_delay);
 
 	while (remaining > 0) {
 		write_bytes = remaining > page_size ? page_size : remaining;
@@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	struct cqspi_st *cqspi;
 	struct resource *res;
 	struct resource *res_ahb;
+	unsigned long data;
 	int ret;
 	int irq;
 
@@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
+	data  = (unsigned long)of_device_get_match_data(dev);
+	if (data & CQSPI_NEEDS_WR_DELAY)
+		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+						   cqspi->master_ref_clk_hz);
 
 	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
 			       pdev->name, cqspi);
@@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #endif
 
 static const struct of_device_id cqspi_dt_ids[] = {
-	{.compatible = "cdns,qspi-nor",},
+	{
+		.compatible = "cdns,qspi-nor",
+		.data = (void *)0,
+	},
+	{
+		.compatible = "ti,k2g-qspi",
+		.data = (void *)CQSPI_NEEDS_WR_DELAY,
+	},
 	{ /* end of table */ }
 };
 
-- 
2.14.1

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
Controller programming sequence, a delay equal to couple of QSPI master
clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
to handle this and set this flag for TI 66AK2G SoC.

[1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v3:
Fix build warnings reported by kbuild test bot.

 drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..5cd5d6f7303f 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -38,6 +38,9 @@
 #define CQSPI_NAME			"cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT		16
 
+/* Quirks */
+#define CQSPI_NEEDS_WR_DELAY		BIT(0)
+
 struct cqspi_st;
 
 struct cqspi_flash_pdata {
@@ -76,6 +79,7 @@ struct cqspi_st {
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			trigger_address;
+	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
 	reinit_completion(&cqspi->transfer_complete);
 	writel(CQSPI_REG_INDIRECTWR_START_MASK,
 	       reg_base + CQSPI_REG_INDIRECTWR);
+	/*
+	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
+	 * Controller programming sequence, couple of cycles of
+	 * QSPI_REF_CLK delay is required for the above bit to
+	 * be internally synchronized by the QSPI module. Provide 5
+	 * cycles of delay.
+	 */
+	if (cqspi->wr_delay)
+		ndelay(cqspi->wr_delay);
 
 	while (remaining > 0) {
 		write_bytes = remaining > page_size ? page_size : remaining;
@@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	struct cqspi_st *cqspi;
 	struct resource *res;
 	struct resource *res_ahb;
+	unsigned long data;
 	int ret;
 	int irq;
 
@@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
+	data  = (unsigned long)of_device_get_match_data(dev);
+	if (data & CQSPI_NEEDS_WR_DELAY)
+		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+						   cqspi->master_ref_clk_hz);
 
 	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
 			       pdev->name, cqspi);
@@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #endif
 
 static const struct of_device_id cqspi_dt_ids[] = {
-	{.compatible = "cdns,qspi-nor",},
+	{
+		.compatible = "cdns,qspi-nor",
+		.data = (void *)0,
+	},
+	{
+		.compatible = "ti,k2g-qspi",
+		.data = (void *)CQSPI_NEEDS_WR_DELAY,
+	},
 	{ /* end of table */ }
 };
 
-- 
2.14.1

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Marek Vasut]

On 09/24/2017 12:59 PM, Vignesh R wrote:
> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
> Controller programming sequence, a delay equal to couple of QSPI master
> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
> to handle this and set this flag for TI 66AK2G SoC.
> 
> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Is this TI specific or is this controller property ? I wouldn't be
surprised of the later ...

> ---
> 
> v3:
> Fix build warnings reported by kbuild test bot.
> 
>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e0327a..5cd5d6f7303f 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -38,6 +38,9 @@
>  #define CQSPI_NAME			"cadence-qspi"
>  #define CQSPI_MAX_CHIPSELECT		16
>  
> +/* Quirks */
> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
> +
>  struct cqspi_st;
>  
>  struct cqspi_flash_pdata {
> @@ -76,6 +79,7 @@ struct cqspi_st {
>  	u32			fifo_depth;
>  	u32			fifo_width;
>  	u32			trigger_address;
> +	u32			wr_delay;
>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>  };
>  
> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>  	reinit_completion(&cqspi->transfer_complete);
>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>  	       reg_base + CQSPI_REG_INDIRECTWR);
> +	/*
> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
> +	 * Controller programming sequence, couple of cycles of
> +	 * QSPI_REF_CLK delay is required for the above bit to
> +	 * be internally synchronized by the QSPI module. Provide 5
> +	 * cycles of delay.
> +	 */
> +	if (cqspi->wr_delay)
> +		ndelay(cqspi->wr_delay);
>  
>  	while (remaining > 0) {
>  		write_bytes = remaining > page_size ? page_size : remaining;
> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	struct cqspi_st *cqspi;
>  	struct resource *res;
>  	struct resource *res_ahb;
> +	unsigned long data;
>  	int ret;
>  	int irq;
>  
> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>  	}
>  
>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> +	data  = (unsigned long)of_device_get_match_data(dev);
> +	if (data & CQSPI_NEEDS_WR_DELAY)
> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
> +						   cqspi->master_ref_clk_hz);
>  
>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>  			       pdev->name, cqspi);
> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>  #endif
>  
>  static const struct of_device_id cqspi_dt_ids[] = {
> -	{.compatible = "cdns,qspi-nor",},
> +	{
> +		.compatible = "cdns,qspi-nor",
> +		.data = (void *)0,
> +	},
> +	{
> +		.compatible = "ti,k2g-qspi",
> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
> +	},
>  	{ /* end of table */ }
>  };
>  
> 


-- 
Best regards,
Marek Vasut

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Marek Vasut]

On 09/24/2017 12:59 PM, Vignesh R wrote:
> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
> Controller programming sequence, a delay equal to couple of QSPI master
> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
> to handle this and set this flag for TI 66AK2G SoC.
> 
> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Is this TI specific or is this controller property ? I wouldn't be
surprised of the later ...

> ---
> 
> v3:
> Fix build warnings reported by kbuild test bot.
> 
>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e0327a..5cd5d6f7303f 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -38,6 +38,9 @@
>  #define CQSPI_NAME			"cadence-qspi"
>  #define CQSPI_MAX_CHIPSELECT		16
>  
> +/* Quirks */
> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
> +
>  struct cqspi_st;
>  
>  struct cqspi_flash_pdata {
> @@ -76,6 +79,7 @@ struct cqspi_st {
>  	u32			fifo_depth;
>  	u32			fifo_width;
>  	u32			trigger_address;
> +	u32			wr_delay;
>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>  };
>  
> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>  	reinit_completion(&cqspi->transfer_complete);
>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>  	       reg_base + CQSPI_REG_INDIRECTWR);
> +	/*
> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
> +	 * Controller programming sequence, couple of cycles of
> +	 * QSPI_REF_CLK delay is required for the above bit to
> +	 * be internally synchronized by the QSPI module. Provide 5
> +	 * cycles of delay.
> +	 */
> +	if (cqspi->wr_delay)
> +		ndelay(cqspi->wr_delay);
>  
>  	while (remaining > 0) {
>  		write_bytes = remaining > page_size ? page_size : remaining;
> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	struct cqspi_st *cqspi;
>  	struct resource *res;
>  	struct resource *res_ahb;
> +	unsigned long data;
>  	int ret;
>  	int irq;
>  
> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>  	}
>  
>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> +	data  = (unsigned long)of_device_get_match_data(dev);
> +	if (data & CQSPI_NEEDS_WR_DELAY)
> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
> +						   cqspi->master_ref_clk_hz);
>  
>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>  			       pdev->name, cqspi);
> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>  #endif
>  
>  static const struct of_device_id cqspi_dt_ids[] = {
> -	{.compatible = "cdns,qspi-nor",},
> +	{
> +		.compatible = "cdns,qspi-nor",
> +		.data = (void *)0,
> +	},
> +	{
> +		.compatible = "ti,k2g-qspi",
> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
> +	},
>  	{ /* end of table */ }
>  };
>  
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 5:29 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>> Controller programming sequence, a delay equal to couple of QSPI master
>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>> to handle this and set this flag for TI 66AK2G SoC.
>>
>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
> Is this TI specific or is this controller property ? I wouldn't be
> surprised of the later ...

I am not sure, there is no generic public documentation by cadence for
this IP. TI TRM clearly states this delay is required and I have
verified it practically that this delay is indeed needed.
But current user of this IP, socfpga does not seem to mention anything
about it. So, I guess its TI specific quirk.

> 
>> ---
>>
>> v3:
>> Fix build warnings reported by kbuild test bot.
>>
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..5cd5d6f7303f 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -38,6 +38,9 @@
>>  #define CQSPI_NAME			"cadence-qspi"
>>  #define CQSPI_MAX_CHIPSELECT		16
>>  
>> +/* Quirks */
>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>> +
>>  struct cqspi_st;
>>  
>>  struct cqspi_flash_pdata {
>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>  	u32			fifo_depth;
>>  	u32			fifo_width;
>>  	u32			trigger_address;
>> +	u32			wr_delay;
>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>  };
>>  
>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>  	reinit_completion(&cqspi->transfer_complete);
>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>> +	/*
>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>> +	 * Controller programming sequence, couple of cycles of
>> +	 * QSPI_REF_CLK delay is required for the above bit to
>> +	 * be internally synchronized by the QSPI module. Provide 5
>> +	 * cycles of delay.
>> +	 */
>> +	if (cqspi->wr_delay)
>> +		ndelay(cqspi->wr_delay);
>>  
>>  	while (remaining > 0) {
>>  		write_bytes = remaining > page_size ? page_size : remaining;
>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	struct cqspi_st *cqspi;
>>  	struct resource *res;
>>  	struct resource *res_ahb;
>> +	unsigned long data;
>>  	int ret;
>>  	int irq;
>>  
>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>> +	data  = (unsigned long)of_device_get_match_data(dev);
>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>> +						   cqspi->master_ref_clk_hz);
>>  
>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>  			       pdev->name, cqspi);
>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>  #endif
>>  
>>  static const struct of_device_id cqspi_dt_ids[] = {
>> -	{.compatible = "cdns,qspi-nor",},
>> +	{
>> +		.compatible = "cdns,qspi-nor",
>> +		.data = (void *)0,
>> +	},
>> +	{
>> +		.compatible = "ti,k2g-qspi",
>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>> +	},
>>  	{ /* end of table */ }
>>  };
>>  
>>
> 
> 

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 5:29 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>> Controller programming sequence, a delay equal to couple of QSPI master
>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>> to handle this and set this flag for TI 66AK2G SoC.
>>
>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
> Is this TI specific or is this controller property ? I wouldn't be
> surprised of the later ...

I am not sure, there is no generic public documentation by cadence for
this IP. TI TRM clearly states this delay is required and I have
verified it practically that this delay is indeed needed.
But current user of this IP, socfpga does not seem to mention anything
about it. So, I guess its TI specific quirk.

> 
>> ---
>>
>> v3:
>> Fix build warnings reported by kbuild test bot.
>>
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..5cd5d6f7303f 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -38,6 +38,9 @@
>>  #define CQSPI_NAME			"cadence-qspi"
>>  #define CQSPI_MAX_CHIPSELECT		16
>>  
>> +/* Quirks */
>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>> +
>>  struct cqspi_st;
>>  
>>  struct cqspi_flash_pdata {
>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>  	u32			fifo_depth;
>>  	u32			fifo_width;
>>  	u32			trigger_address;
>> +	u32			wr_delay;
>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>  };
>>  
>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>  	reinit_completion(&cqspi->transfer_complete);
>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>> +	/*
>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>> +	 * Controller programming sequence, couple of cycles of
>> +	 * QSPI_REF_CLK delay is required for the above bit to
>> +	 * be internally synchronized by the QSPI module. Provide 5
>> +	 * cycles of delay.
>> +	 */
>> +	if (cqspi->wr_delay)
>> +		ndelay(cqspi->wr_delay);
>>  
>>  	while (remaining > 0) {
>>  		write_bytes = remaining > page_size ? page_size : remaining;
>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	struct cqspi_st *cqspi;
>>  	struct resource *res;
>>  	struct resource *res_ahb;
>> +	unsigned long data;
>>  	int ret;
>>  	int irq;
>>  
>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>> +	data  = (unsigned long)of_device_get_match_data(dev);
>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>> +						   cqspi->master_ref_clk_hz);
>>  
>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>  			       pdev->name, cqspi);
>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>  #endif
>>  
>>  static const struct of_device_id cqspi_dt_ids[] = {
>> -	{.compatible = "cdns,qspi-nor",},
>> +	{
>> +		.compatible = "cdns,qspi-nor",
>> +		.data = (void *)0,
>> +	},
>> +	{
>> +		.compatible = "ti,k2g-qspi",
>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>> +	},
>>  	{ /* end of table */ }
>>  };
>>  
>>
> 
> 

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 5:29 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>> Controller programming sequence, a delay equal to couple of QSPI master
>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>> to handle this and set this flag for TI 66AK2G SoC.
>>
>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>
>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
> 
> Is this TI specific or is this controller property ? I wouldn't be
> surprised of the later ...

I am not sure, there is no generic public documentation by cadence for
this IP. TI TRM clearly states this delay is required and I have
verified it practically that this delay is indeed needed.
But current user of this IP, socfpga does not seem to mention anything
about it. So, I guess its TI specific quirk.

> 
>> ---
>>
>> v3:
>> Fix build warnings reported by kbuild test bot.
>>
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..5cd5d6f7303f 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -38,6 +38,9 @@
>>  #define CQSPI_NAME			"cadence-qspi"
>>  #define CQSPI_MAX_CHIPSELECT		16
>>  
>> +/* Quirks */
>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>> +
>>  struct cqspi_st;
>>  
>>  struct cqspi_flash_pdata {
>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>  	u32			fifo_depth;
>>  	u32			fifo_width;
>>  	u32			trigger_address;
>> +	u32			wr_delay;
>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>  };
>>  
>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>  	reinit_completion(&cqspi->transfer_complete);
>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>> +	/*
>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>> +	 * Controller programming sequence, couple of cycles of
>> +	 * QSPI_REF_CLK delay is required for the above bit to
>> +	 * be internally synchronized by the QSPI module. Provide 5
>> +	 * cycles of delay.
>> +	 */
>> +	if (cqspi->wr_delay)
>> +		ndelay(cqspi->wr_delay);
>>  
>>  	while (remaining > 0) {
>>  		write_bytes = remaining > page_size ? page_size : remaining;
>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	struct cqspi_st *cqspi;
>>  	struct resource *res;
>>  	struct resource *res_ahb;
>> +	unsigned long data;
>>  	int ret;
>>  	int irq;
>>  
>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>> +	data  = (unsigned long)of_device_get_match_data(dev);
>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>> +						   cqspi->master_ref_clk_hz);
>>  
>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>  			       pdev->name, cqspi);
>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>  #endif
>>  
>>  static const struct of_device_id cqspi_dt_ids[] = {
>> -	{.compatible = "cdns,qspi-nor",},
>> +	{
>> +		.compatible = "cdns,qspi-nor",
>> +		.data = (void *)0,
>> +	},
>> +	{
>> +		.compatible = "ti,k2g-qspi",
>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>> +	},
>>  	{ /* end of table */ }
>>  };
>>  
>>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Marek Vasut]

On 09/24/2017 02:33 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>> Controller programming sequence, a delay equal to couple of QSPI master
>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>> to handle this and set this flag for TI 66AK2G SoC.
>>>
>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>
>> Is this TI specific or is this controller property ? I wouldn't be
>> surprised of the later ...
> 
> I am not sure, there is no generic public documentation by cadence for
> this IP. TI TRM clearly states this delay is required and I have
> verified it practically that this delay is indeed needed.
> But current user of this IP, socfpga does not seem to mention anything
> about it. So, I guess its TI specific quirk.

OK, let's go with that then. I didn't observe any stability issues with
SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
of frequencies does the quirk become relevant ?

>>
>>> ---
>>>
>>> v3:
>>> Fix build warnings reported by kbuild test bot.
>>>
>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -38,6 +38,9 @@
>>>  #define CQSPI_NAME			"cadence-qspi"
>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>  
>>> +/* Quirks */
>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>> +
>>>  struct cqspi_st;
>>>  
>>>  struct cqspi_flash_pdata {
>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>  	u32			fifo_depth;
>>>  	u32			fifo_width;
>>>  	u32			trigger_address;
>>> +	u32			wr_delay;
>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>  };
>>>  
>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>  	reinit_completion(&cqspi->transfer_complete);
>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>> +	/*
>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>> +	 * Controller programming sequence, couple of cycles of
>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>> +	 * cycles of delay.
>>> +	 */
>>> +	if (cqspi->wr_delay)
>>> +		ndelay(cqspi->wr_delay);
>>>  
>>>  	while (remaining > 0) {
>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	struct cqspi_st *cqspi;
>>>  	struct resource *res;
>>>  	struct resource *res_ahb;
>>> +	unsigned long data;
>>>  	int ret;
>>>  	int irq;
>>>  
>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	}
>>>  
>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>> +						   cqspi->master_ref_clk_hz);
>>>  
>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>  			       pdev->name, cqspi);
>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>  #endif
>>>  
>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>> -	{.compatible = "cdns,qspi-nor",},
>>> +	{
>>> +		.compatible = "cdns,qspi-nor",
>>> +		.data = (void *)0,
>>> +	},
>>> +	{
>>> +		.compatible = "ti,k2g-qspi",
>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>> +	},
>>>  	{ /* end of table */ }
>>>  };
>>>  
>>>
>>
>>


-- 
Best regards,
Marek Vasut

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Marek Vasut]

On 09/24/2017 02:33 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>> Controller programming sequence, a delay equal to couple of QSPI master
>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>> to handle this and set this flag for TI 66AK2G SoC.
>>>
>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>
>> Is this TI specific or is this controller property ? I wouldn't be
>> surprised of the later ...
> 
> I am not sure, there is no generic public documentation by cadence for
> this IP. TI TRM clearly states this delay is required and I have
> verified it practically that this delay is indeed needed.
> But current user of this IP, socfpga does not seem to mention anything
> about it. So, I guess its TI specific quirk.

OK, let's go with that then. I didn't observe any stability issues with
SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
of frequencies does the quirk become relevant ?

>>
>>> ---
>>>
>>> v3:
>>> Fix build warnings reported by kbuild test bot.
>>>
>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -38,6 +38,9 @@
>>>  #define CQSPI_NAME			"cadence-qspi"
>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>  
>>> +/* Quirks */
>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>> +
>>>  struct cqspi_st;
>>>  
>>>  struct cqspi_flash_pdata {
>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>  	u32			fifo_depth;
>>>  	u32			fifo_width;
>>>  	u32			trigger_address;
>>> +	u32			wr_delay;
>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>  };
>>>  
>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>  	reinit_completion(&cqspi->transfer_complete);
>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>> +	/*
>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>> +	 * Controller programming sequence, couple of cycles of
>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>> +	 * cycles of delay.
>>> +	 */
>>> +	if (cqspi->wr_delay)
>>> +		ndelay(cqspi->wr_delay);
>>>  
>>>  	while (remaining > 0) {
>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	struct cqspi_st *cqspi;
>>>  	struct resource *res;
>>>  	struct resource *res_ahb;
>>> +	unsigned long data;
>>>  	int ret;
>>>  	int irq;
>>>  
>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	}
>>>  
>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>> +						   cqspi->master_ref_clk_hz);
>>>  
>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>  			       pdev->name, cqspi);
>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>  #endif
>>>  
>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>> -	{.compatible = "cdns,qspi-nor",},
>>> +	{
>>> +		.compatible = "cdns,qspi-nor",
>>> +		.data = (void *)0,
>>> +	},
>>> +	{
>>> +		.compatible = "ti,k2g-qspi",
>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>> +	},
>>>  	{ /* end of table */ }
>>>  };
>>>  
>>>
>>
>>


-- 
Best regards,
Marek Vasut

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Marek Vasut]

On 09/24/2017 02:33 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>> Controller programming sequence, a delay equal to couple of QSPI master
>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>> to handle this and set this flag for TI 66AK2G SoC.
>>>
>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>
>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>
>> Is this TI specific or is this controller property ? I wouldn't be
>> surprised of the later ...
> 
> I am not sure, there is no generic public documentation by cadence for
> this IP. TI TRM clearly states this delay is required and I have
> verified it practically that this delay is indeed needed.
> But current user of this IP, socfpga does not seem to mention anything
> about it. So, I guess its TI specific quirk.

OK, let's go with that then. I didn't observe any stability issues with
SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
of frequencies does the quirk become relevant ?

>>
>>> ---
>>>
>>> v3:
>>> Fix build warnings reported by kbuild test bot.
>>>
>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -38,6 +38,9 @@
>>>  #define CQSPI_NAME			"cadence-qspi"
>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>  
>>> +/* Quirks */
>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>> +
>>>  struct cqspi_st;
>>>  
>>>  struct cqspi_flash_pdata {
>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>  	u32			fifo_depth;
>>>  	u32			fifo_width;
>>>  	u32			trigger_address;
>>> +	u32			wr_delay;
>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>  };
>>>  
>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>  	reinit_completion(&cqspi->transfer_complete);
>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>> +	/*
>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>> +	 * Controller programming sequence, couple of cycles of
>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>> +	 * cycles of delay.
>>> +	 */
>>> +	if (cqspi->wr_delay)
>>> +		ndelay(cqspi->wr_delay);
>>>  
>>>  	while (remaining > 0) {
>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	struct cqspi_st *cqspi;
>>>  	struct resource *res;
>>>  	struct resource *res_ahb;
>>> +	unsigned long data;
>>>  	int ret;
>>>  	int irq;
>>>  
>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>  	}
>>>  
>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>> +						   cqspi->master_ref_clk_hz);
>>>  
>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>  			       pdev->name, cqspi);
>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>  #endif
>>>  
>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>> -	{.compatible = "cdns,qspi-nor",},
>>> +	{
>>> +		.compatible = "cdns,qspi-nor",
>>> +		.data = (void *)0,
>>> +	},
>>> +	{
>>> +		.compatible = "ti,k2g-qspi",
>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>> +	},
>>>  	{ /* end of table */ }
>>>  };
>>>  
>>>
>>
>>


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 6:43 PM, Marek Vasut wrote:
> On 09/24/2017 02:33 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>>> Controller programming sequence, a delay equal to couple of QSPI master
>>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>>> to handle this and set this flag for TI 66AK2G SoC.
>>>>
>>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>
>>> Is this TI specific or is this controller property ? I wouldn't be
>>> surprised of the later ...
>>
>> I am not sure, there is no generic public documentation by cadence for
>> this IP. TI TRM clearly states this delay is required and I have
>> verified it practically that this delay is indeed needed.
>> But current user of this IP, socfpga does not seem to mention anything
>> about it. So, I guess its TI specific quirk.
> 
> OK, let's go with that then. I didn't observe any stability issues with
> SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
> of frequencies does the quirk become relevant ?
> 

Actually, delay is tied to QSPI master clk rate(not SPI bus clk rate).
It runs at 384MHz. Changing SPI bus rate has no effect.

Regards
Vignesh

>>>
>>>> ---
>>>>
>>>> v3:
>>>> Fix build warnings reported by kbuild test bot.
>>>>
>>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> @@ -38,6 +38,9 @@
>>>>  #define CQSPI_NAME			"cadence-qspi"
>>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>>  
>>>> +/* Quirks */
>>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>>> +
>>>>  struct cqspi_st;
>>>>  
>>>>  struct cqspi_flash_pdata {
>>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>>  	u32			fifo_depth;
>>>>  	u32			fifo_width;
>>>>  	u32			trigger_address;
>>>> +	u32			wr_delay;
>>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>>  };
>>>>  
>>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>>  	reinit_completion(&cqspi->transfer_complete);
>>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>>> +	/*
>>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>>> +	 * Controller programming sequence, couple of cycles of
>>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>>> +	 * cycles of delay.
>>>> +	 */
>>>> +	if (cqspi->wr_delay)
>>>> +		ndelay(cqspi->wr_delay);
>>>>  
>>>>  	while (remaining > 0) {
>>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	struct cqspi_st *cqspi;
>>>>  	struct resource *res;
>>>>  	struct resource *res_ahb;
>>>> +	unsigned long data;
>>>>  	int ret;
>>>>  	int irq;
>>>>  
>>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	}
>>>>  
>>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>>> +						   cqspi->master_ref_clk_hz);
>>>>  
>>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>>  			       pdev->name, cqspi);
>>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>>  #endif
>>>>  
>>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>>> -	{.compatible = "cdns,qspi-nor",},
>>>> +	{
>>>> +		.compatible = "cdns,qspi-nor",
>>>> +		.data = (void *)0,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "ti,k2g-qspi",
>>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>>> +	},
>>>>  	{ /* end of table */ }
>>>>  };
>>>>  
>>>>
>>>
>>>
> 
> 

[PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 6:43 PM, Marek Vasut wrote:
> On 09/24/2017 02:33 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>>> Controller programming sequence, a delay equal to couple of QSPI master
>>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>>> to handle this and set this flag for TI 66AK2G SoC.
>>>>
>>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>
>>> Is this TI specific or is this controller property ? I wouldn't be
>>> surprised of the later ...
>>
>> I am not sure, there is no generic public documentation by cadence for
>> this IP. TI TRM clearly states this delay is required and I have
>> verified it practically that this delay is indeed needed.
>> But current user of this IP, socfpga does not seem to mention anything
>> about it. So, I guess its TI specific quirk.
> 
> OK, let's go with that then. I didn't observe any stability issues with
> SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
> of frequencies does the quirk become relevant ?
> 

Actually, delay is tied to QSPI master clk rate(not SPI bus clk rate).
It runs at 384MHz. Changing SPI bus rate has no effect.

Regards
Vignesh

>>>
>>>> ---
>>>>
>>>> v3:
>>>> Fix build warnings reported by kbuild test bot.
>>>>
>>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> @@ -38,6 +38,9 @@
>>>>  #define CQSPI_NAME			"cadence-qspi"
>>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>>  
>>>> +/* Quirks */
>>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>>> +
>>>>  struct cqspi_st;
>>>>  
>>>>  struct cqspi_flash_pdata {
>>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>>  	u32			fifo_depth;
>>>>  	u32			fifo_width;
>>>>  	u32			trigger_address;
>>>> +	u32			wr_delay;
>>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>>  };
>>>>  
>>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>>  	reinit_completion(&cqspi->transfer_complete);
>>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>>> +	/*
>>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>>> +	 * Controller programming sequence, couple of cycles of
>>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>>> +	 * cycles of delay.
>>>> +	 */
>>>> +	if (cqspi->wr_delay)
>>>> +		ndelay(cqspi->wr_delay);
>>>>  
>>>>  	while (remaining > 0) {
>>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	struct cqspi_st *cqspi;
>>>>  	struct resource *res;
>>>>  	struct resource *res_ahb;
>>>> +	unsigned long data;
>>>>  	int ret;
>>>>  	int irq;
>>>>  
>>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	}
>>>>  
>>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>>> +						   cqspi->master_ref_clk_hz);
>>>>  
>>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>>  			       pdev->name, cqspi);
>>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>>  #endif
>>>>  
>>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>>> -	{.compatible = "cdns,qspi-nor",},
>>>> +	{
>>>> +		.compatible = "cdns,qspi-nor",
>>>> +		.data = (void *)0,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "ti,k2g-qspi",
>>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>>> +	},
>>>>  	{ /* end of table */ }
>>>>  };
>>>>  
>>>>
>>>
>>>
> 
> 

Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence

[Vignesh R]

On 9/24/2017 6:43 PM, Marek Vasut wrote:
> On 09/24/2017 02:33 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>>> Controller programming sequence, a delay equal to couple of QSPI master
>>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>>> to handle this and set this flag for TI 66AK2G SoC.
>>>>
>>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>>
>>> Is this TI specific or is this controller property ? I wouldn't be
>>> surprised of the later ...
>>
>> I am not sure, there is no generic public documentation by cadence for
>> this IP. TI TRM clearly states this delay is required and I have
>> verified it practically that this delay is indeed needed.
>> But current user of this IP, socfpga does not seem to mention anything
>> about it. So, I guess its TI specific quirk.
> 
> OK, let's go with that then. I didn't observe any stability issues with
> SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
> of frequencies does the quirk become relevant ?
> 

Actually, delay is tied to QSPI master clk rate(not SPI bus clk rate).
It runs at 384MHz. Changing SPI bus rate has no effect.

Regards
Vignesh

>>>
>>>> ---
>>>>
>>>> v3:
>>>> Fix build warnings reported by kbuild test bot.
>>>>
>>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++++++++++++++++++++++++++-
>>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> @@ -38,6 +38,9 @@
>>>>  #define CQSPI_NAME			"cadence-qspi"
>>>>  #define CQSPI_MAX_CHIPSELECT		16
>>>>  
>>>> +/* Quirks */
>>>> +#define CQSPI_NEEDS_WR_DELAY		BIT(0)
>>>> +
>>>>  struct cqspi_st;
>>>>  
>>>>  struct cqspi_flash_pdata {
>>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>>>  	u32			fifo_depth;
>>>>  	u32			fifo_width;
>>>>  	u32			trigger_address;
>>>> +	u32			wr_delay;
>>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>>  };
>>>>  
>>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor,
>>>>  	reinit_completion(&cqspi->transfer_complete);
>>>>  	writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>>  	       reg_base + CQSPI_REG_INDIRECTWR);
>>>> +	/*
>>>> +	 * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>>> +	 * Controller programming sequence, couple of cycles of
>>>> +	 * QSPI_REF_CLK delay is required for the above bit to
>>>> +	 * be internally synchronized by the QSPI module. Provide 5
>>>> +	 * cycles of delay.
>>>> +	 */
>>>> +	if (cqspi->wr_delay)
>>>> +		ndelay(cqspi->wr_delay);
>>>>  
>>>>  	while (remaining > 0) {
>>>>  		write_bytes = remaining > page_size ? page_size : remaining;
>>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	struct cqspi_st *cqspi;
>>>>  	struct resource *res;
>>>>  	struct resource *res_ahb;
>>>> +	unsigned long data;
>>>>  	int ret;
>>>>  	int irq;
>>>>  
>>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>>>  	}
>>>>  
>>>>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>>> +	data  = (unsigned long)of_device_get_match_data(dev);
>>>> +	if (data & CQSPI_NEEDS_WR_DELAY)
>>>> +		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>>> +						   cqspi->master_ref_clk_hz);
>>>>  
>>>>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>>  			       pdev->name, cqspi);
>>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>>  #endif
>>>>  
>>>>  static const struct of_device_id cqspi_dt_ids[] = {
>>>> -	{.compatible = "cdns,qspi-nor",},
>>>> +	{
>>>> +		.compatible = "cdns,qspi-nor",
>>>> +		.data = (void *)0,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "ti,k2g-qspi",
>>>> +		.data = (void *)CQSPI_NEEDS_WR_DELAY,
>>>> +	},
>>>>  	{ /* end of table */ }
>>>>  };
>>>>  
>>>>
>>>
>>>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/5] mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Add DT parameter cdns,rclk-en to help enable adapted loop-back circuit
for boards which do have QSPI return clock provided. Update binding
documentation for the same.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index 7dbe3bd9ac56..bb2075df9b38 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -16,6 +16,9 @@ Required properties:
 
 Optional properties:
 - cdns,is-decoded-cs : Flag to indicate whether decoder is used or not.
+- cdns,rclk-en : Flag to indicate that QSPI return clock is used to latch
+  the read data rather than the QSPI clock. Make sure that QSPI return
+  clock is populated on the board before using this property.
 
 Optional subnodes:
 Subnodes of the Cadence Quad SPI controller are spi slave nodes with additional
-- 
2.14.1

[PATCH v3 3/5] mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Add DT parameter cdns,rclk-en to help enable adapted loop-back circuit
for boards which do have QSPI return clock provided. Update binding
documentation for the same.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index 7dbe3bd9ac56..bb2075df9b38 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -16,6 +16,9 @@ Required properties:
 
 Optional properties:
 - cdns,is-decoded-cs : Flag to indicate whether decoder is used or not.
+- cdns,rclk-en : Flag to indicate that QSPI return clock is used to latch
+  the read data rather than the QSPI clock. Make sure that QSPI return
+  clock is populated on the board before using this property.
 
 Optional subnodes:
 Subnodes of the Cadence Quad SPI controller are spi slave nodes with additional
-- 
2.14.1

[PATCH v3 3/5] mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Add DT parameter cdns,rclk-en to help enable adapted loop-back circuit
for boards which do have QSPI return clock provided. Update binding
documentation for the same.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index 7dbe3bd9ac56..bb2075df9b38 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -16,6 +16,9 @@ Required properties:
 
 Optional properties:
 - cdns,is-decoded-cs : Flag to indicate whether decoder is used or not.
+- cdns,rclk-en : Flag to indicate that QSPI return clock is used to latch
+  the read data rather than the QSPI clock. Make sure that QSPI return
+  clock is populated on the board before using this property.
 
 Optional subnodes:
 Subnodes of the Cadence Quad SPI controller are spi slave nodes with additional
-- 
2.14.1

[PATCH v3 4/5] mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Based on DT parameter cdns,rclk-en enable adapted loop-back circuit
for boards which do have QSPI return clock provided.
This patch also modifies cqspi_readdata_capture() function's bypass
parameter to bool to match how its used in the function.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 5cd5d6f7303f..d9629e8f4798 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -78,6 +78,7 @@ struct cqspi_st {
 	bool			is_decoded_cs;
 	u32			fifo_depth;
 	u32			fifo_width;
+	bool			rclk_en;
 	u32			trigger_address;
 	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
@@ -788,7 +789,7 @@ static void cqspi_config_baudrate_div(struct cqspi_st *cqspi)
 }
 
 static void cqspi_readdata_capture(struct cqspi_st *cqspi,
-				   const unsigned int bypass,
+				   const bool bypass,
 				   const unsigned int delay)
 {
 	void __iomem *reg_base = cqspi->iobase;
@@ -852,7 +853,8 @@ static void cqspi_configure(struct spi_nor *nor)
 		cqspi->sclk = sclk;
 		cqspi_config_baudrate_div(cqspi);
 		cqspi_delay(nor);
-		cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
+		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
+				       f_pdata->read_delay);
 	}
 
 	if (switch_cs || switch_ck)
@@ -1049,6 +1051,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 4/5] mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Based on DT parameter cdns,rclk-en enable adapted loop-back circuit
for boards which do have QSPI return clock provided.
This patch also modifies cqspi_readdata_capture() function's bypass
parameter to bool to match how its used in the function.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 5cd5d6f7303f..d9629e8f4798 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -78,6 +78,7 @@ struct cqspi_st {
 	bool			is_decoded_cs;
 	u32			fifo_depth;
 	u32			fifo_width;
+	bool			rclk_en;
 	u32			trigger_address;
 	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
@@ -788,7 +789,7 @@ static void cqspi_config_baudrate_div(struct cqspi_st *cqspi)
 }
 
 static void cqspi_readdata_capture(struct cqspi_st *cqspi,
-				   const unsigned int bypass,
+				   const bool bypass,
 				   const unsigned int delay)
 {
 	void __iomem *reg_base = cqspi->iobase;
@@ -852,7 +853,8 @@ static void cqspi_configure(struct spi_nor *nor)
 		cqspi->sclk = sclk;
 		cqspi_config_baudrate_div(cqspi);
 		cqspi_delay(nor);
-		cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
+		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
+				       f_pdata->read_delay);
 	}
 
 	if (switch_cs || switch_ck)
@@ -1049,6 +1051,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 4/5] mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock circuit

[Vignesh R]

Cadence QSPI IP has a adapted loop-back circuit which can be enabled by
setting BYPASS field to 0 in READCAPTURE register. It enables use of
QSPI return clock to latch the data rather than the internal QSPI
reference clock. For high speed operations, adapted loop-back circuit
using QSPI return clock helps to increase data valid window.

Based on DT parameter cdns,rclk-en enable adapted loop-back circuit
for boards which do have QSPI return clock provided.
This patch also modifies cqspi_readdata_capture() function's bypass
parameter to bool to match how its used in the function.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 5cd5d6f7303f..d9629e8f4798 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -78,6 +78,7 @@ struct cqspi_st {
 	bool			is_decoded_cs;
 	u32			fifo_depth;
 	u32			fifo_width;
+	bool			rclk_en;
 	u32			trigger_address;
 	u32			wr_delay;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
@@ -788,7 +789,7 @@ static void cqspi_config_baudrate_div(struct cqspi_st *cqspi)
 }
 
 static void cqspi_readdata_capture(struct cqspi_st *cqspi,
-				   const unsigned int bypass,
+				   const bool bypass,
 				   const unsigned int delay)
 {
 	void __iomem *reg_base = cqspi->iobase;
@@ -852,7 +853,8 @@ static void cqspi_configure(struct spi_nor *nor)
 		cqspi->sclk = sclk;
 		cqspi_config_baudrate_div(cqspi);
 		cqspi_delay(nor);
-		cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
+		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
+				       f_pdata->read_delay);
 	}
 
 	if (switch_cs || switch_ck)
@@ -1049,6 +1051,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Add pm_runtime* calls to cadence-quadspi driver. This is required to
switch on QSPI power domain on TI 66AK2G SoC during probe.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index d9629e8f4798..2c8e6226d267 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -31,6 +31,7 @@
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
@@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return ret;
+	}
+
 	ret = clk_prepare_enable(cqspi->clk);
 	if (ret) {
 		dev_err(dev, "Cannot enable QSPI clock.\n");
@@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(cqspi->clk);
 
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Add pm_runtime* calls to cadence-quadspi driver. This is required to
switch on QSPI power domain on TI 66AK2G SoC during probe.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index d9629e8f4798..2c8e6226d267 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -31,6 +31,7 @@
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
@@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return ret;
+	}
+
 	ret = clk_prepare_enable(cqspi->clk);
 	if (ret) {
 		dev_err(dev, "Cannot enable QSPI clock.\n");
@@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(cqspi->clk);
 
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Add pm_runtime* calls to cadence-quadspi driver. This is required to
switch on QSPI power domain on TI 66AK2G SoC during probe.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index d9629e8f4798..2c8e6226d267 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -31,6 +31,7 @@
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
@@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return ret;
+	}
+
 	ret = clk_prepare_enable(cqspi->clk);
 	if (ret) {
 		dev_err(dev, "Cannot enable QSPI clock.\n");
@@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(cqspi->clk);
 
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
-- 
2.14.1

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 12:59 PM, Vignesh R wrote:
> Add pm_runtime* calls to cadence-quadspi driver. This is required to
> switch on QSPI power domain on TI 66AK2G SoC during probe.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Are you planning to add some more fine-grained PM control later?

> ---
>  drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index d9629e8f4798..2c8e6226d267 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -31,6 +31,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/sched.h>
>  #include <linux/spi/spi.h>
>  #include <linux/timer.h>
> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
>  		return -ENXIO;
>  	}
>  
> +	pm_runtime_enable(&pdev->dev);
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		return ret;
> +	}
> +
>  	ret = clk_prepare_enable(cqspi->clk);
>  	if (ret) {
>  		dev_err(dev, "Cannot enable QSPI clock.\n");
> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>  
>  	clk_disable_unprepare(cqspi->clk);
>  
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> 


-- 
Best regards,
Marek Vasut

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 12:59 PM, Vignesh R wrote:
> Add pm_runtime* calls to cadence-quadspi driver. This is required to
> switch on QSPI power domain on TI 66AK2G SoC during probe.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Are you planning to add some more fine-grained PM control later?

> ---
>  drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index d9629e8f4798..2c8e6226d267 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -31,6 +31,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/sched.h>
>  #include <linux/spi/spi.h>
>  #include <linux/timer.h>
> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
>  		return -ENXIO;
>  	}
>  
> +	pm_runtime_enable(&pdev->dev);
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		return ret;
> +	}
> +
>  	ret = clk_prepare_enable(cqspi->clk);
>  	if (ret) {
>  		dev_err(dev, "Cannot enable QSPI clock.\n");
> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>  
>  	clk_disable_unprepare(cqspi->clk);
>  
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 5:31 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
> Are you planning to add some more fine-grained PM control later?

Yes, I will need to add fine-grained PM control at some point. But, for
now SoC does not really support low power mode or runtime power saving
option.
The fact that driver still uses clk_prepare_*() calls to enable/disable
clocks instead of pm_*() calls makes it a bit tricky though.

Just figured out I forgot to add cleanup code in error handling path of
probe(). Will fix that and send a v4.

> 
>> ---
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index d9629e8f4798..2c8e6226d267 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/of_device.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>  #include <linux/sched.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/timer.h>
>> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
>>  		return -ENXIO;
>>  	}
>>  
>> +	pm_runtime_enable(&pdev->dev);
>> +	ret = pm_runtime_get_sync(&pdev->dev);
>> +	if (ret < 0) {
>> +		pm_runtime_put_noidle(&pdev->dev);
>> +		return ret;
>> +	}
>> +
>>  	ret = clk_prepare_enable(cqspi->clk);
>>  	if (ret) {
>>  		dev_err(dev, "Cannot enable QSPI clock.\n");
>> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>>  
>>  	clk_disable_unprepare(cqspi->clk);
>>  
>> +	pm_runtime_put_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 5:31 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
> Are you planning to add some more fine-grained PM control later?

Yes, I will need to add fine-grained PM control at some point. But, for
now SoC does not really support low power mode or runtime power saving
option.
The fact that driver still uses clk_prepare_*() calls to enable/disable
clocks instead of pm_*() calls makes it a bit tricky though.

Just figured out I forgot to add cleanup code in error handling path of
probe(). Will fix that and send a v4.

> 
>> ---
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index d9629e8f4798..2c8e6226d267 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/of_device.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>  #include <linux/sched.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/timer.h>
>> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
>>  		return -ENXIO;
>>  	}
>>  
>> +	pm_runtime_enable(&pdev->dev);
>> +	ret = pm_runtime_get_sync(&pdev->dev);
>> +	if (ret < 0) {
>> +		pm_runtime_put_noidle(&pdev->dev);
>> +		return ret;
>> +	}
>> +
>>  	ret = clk_prepare_enable(cqspi->clk);
>>  	if (ret) {
>>  		dev_err(dev, "Cannot enable QSPI clock.\n");
>> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>>  
>>  	clk_disable_unprepare(cqspi->clk);
>>  
>> +	pm_runtime_put_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 5:31 PM, Marek Vasut wrote:
> On 09/24/2017 12:59 PM, Vignesh R wrote:
>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
> Are you planning to add some more fine-grained PM control later?

Yes, I will need to add fine-grained PM control at some point. But, for
now SoC does not really support low power mode or runtime power saving
option.
The fact that driver still uses clk_prepare_*() calls to enable/disable
clocks instead of pm_*() calls makes it a bit tricky though.

Just figured out I forgot to add cleanup code in error handling path of
probe(). Will fix that and send a v4.

> 
>> ---
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index d9629e8f4798..2c8e6226d267 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/of_device.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>  #include <linux/sched.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/timer.h>
>> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
>>  		return -ENXIO;
>>  	}
>>  
>> +	pm_runtime_enable(&pdev->dev);
>> +	ret = pm_runtime_get_sync(&pdev->dev);
>> +	if (ret < 0) {
>> +		pm_runtime_put_noidle(&pdev->dev);
>> +		return ret;
>> +	}
>> +
>>  	ret = clk_prepare_enable(cqspi->clk);
>>  	if (ret) {
>>  		dev_err(dev, "Cannot enable QSPI clock.\n");
>> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>>  
>>  	clk_disable_unprepare(cqspi->clk);
>>  
>> +	pm_runtime_put_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 03:08 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>
>> Are you planning to add some more fine-grained PM control later?
> 
> Yes, I will need to add fine-grained PM control at some point. But, for
> now SoC does not really support low power mode or runtime power saving
> option.
> The fact that driver still uses clk_prepare_*() calls to enable/disable
> clocks instead of pm_*() calls makes it a bit tricky though.
> 
> Just figured out I forgot to add cleanup code in error handling path of
> probe(). Will fix that and send a v4.

OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
either, so it's fine for now.

-- 
Best regards,
Marek Vasut

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 03:08 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>
>> Are you planning to add some more fine-grained PM control later?
> 
> Yes, I will need to add fine-grained PM control at some point. But, for
> now SoC does not really support low power mode or runtime power saving
> option.
> The fact that driver still uses clk_prepare_*() calls to enable/disable
> clocks instead of pm_*() calls makes it a bit tricky though.
> 
> Just figured out I forgot to add cleanup code in error handling path of
> probe(). Will fix that and send a v4.

OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
either, so it's fine for now.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 03:08 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>
>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>
>> Are you planning to add some more fine-grained PM control later?
> 
> Yes, I will need to add fine-grained PM control at some point. But, for
> now SoC does not really support low power mode or runtime power saving
> option.
> The fact that driver still uses clk_prepare_*() calls to enable/disable
> clocks instead of pm_*() calls makes it a bit tricky though.
> 
> Just figured out I forgot to add cleanup code in error handling path of
> probe(). Will fix that and send a v4.

OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
either, so it's fine for now.

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 6:42 PM, Marek Vasut wrote:
> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>
>>> Are you planning to add some more fine-grained PM control later?
>>
>> Yes, I will need to add fine-grained PM control at some point. But, for
>> now SoC does not really support low power mode or runtime power saving
>> option.
>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>> clocks instead of pm_*() calls makes it a bit tricky though.
>>
>> Just figured out I forgot to add cleanup code in error handling path of
>> probe(). Will fix that and send a v4.
> 
> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
> either, so it's fine for now.
> 

Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
if its possible to get rid of clk_*() calls in favor of pm_*() calls.

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 6:42 PM, Marek Vasut wrote:
> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>
>>> Are you planning to add some more fine-grained PM control later?
>>
>> Yes, I will need to add fine-grained PM control at some point. But, for
>> now SoC does not really support low power mode or runtime power saving
>> option.
>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>> clocks instead of pm_*() calls makes it a bit tricky though.
>>
>> Just figured out I forgot to add cleanup code in error handling path of
>> probe(). Will fix that and send a v4.
> 
> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
> either, so it's fine for now.
> 

Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
if its possible to get rid of clk_*() calls in favor of pm_*() calls.

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

On 9/24/2017 6:42 PM, Marek Vasut wrote:
> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>>
>>> Are you planning to add some more fine-grained PM control later?
>>
>> Yes, I will need to add fine-grained PM control at some point. But, for
>> now SoC does not really support low power mode or runtime power saving
>> option.
>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>> clocks instead of pm_*() calls makes it a bit tricky though.
>>
>> Just figured out I forgot to add cleanup code in error handling path of
>> probe(). Will fix that and send a v4.
> 
> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
> either, so it's fine for now.
> 

Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
if its possible to get rid of clk_*() calls in favor of pm_*() calls.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 03:27 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>
>>>
>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>
>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>
>>>> Are you planning to add some more fine-grained PM control later?
>>>
>>> Yes, I will need to add fine-grained PM control at some point. But, for
>>> now SoC does not really support low power mode or runtime power saving
>>> option.
>>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>
>>> Just figured out I forgot to add cleanup code in error handling path of
>>> probe(). Will fix that and send a v4.
>>
>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>> either, so it's fine for now.
>>
> 
> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
> if its possible to get rid of clk_*() calls in favor of pm_*() calls.

Not of the top of my head, sorry. +CC Matthew, he should know.

-- 
Best regards,
Marek Vasut

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/24/2017 03:27 PM, Vignesh R wrote:
> 
> 
> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>
>>>
>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>
>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>
>>>> Are you planning to add some more fine-grained PM control later?
>>>
>>> Yes, I will need to add fine-grained PM control at some point. But, for
>>> now SoC does not really support low power mode or runtime power saving
>>> option.
>>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>
>>> Just figured out I forgot to add cleanup code in error handling path of
>>> probe(). Will fix that and send a v4.
>>
>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>> either, so it's fine for now.
>>
> 
> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
> if its possible to get rid of clk_*() calls in favor of pm_*() calls.

Not of the top of my head, sorry. +CC Matthew, he should know.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[matthew.gerlach]

On Sun, 24 Sep 2017, Marek Vasut wrote:

> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>
>>>>
>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>
>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>>
>>>>> Are you planning to add some more fine-grained PM control later?
>>>>
>>>> Yes, I will need to add fine-grained PM control at some point. But, for
>>>> now SoC does not really support low power mode or runtime power saving
>>>> option.
>>>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>
>>>> Just figured out I forgot to add cleanup code in error handling path of
>>>> probe(). Will fix that and send a v4.
>>>
>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>> either, so it's fine for now.
>>>
>>
>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>
> Not of the top of my head, sorry. +CC Matthew, he should know.

I am not an expert at the clock framework nor the power management, but I
did ask around a bit.  No one I asked was planning to change the clk_*()
calls to pm_*() call, but the feedback was that it would be a good idea.

Matthew Gerlach


>
> -- 
> Best regards,
> Marek Vasut
>

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[matthew.gerlach at linux.intel.com]

On Sun, 24 Sep 2017, Marek Vasut wrote:

> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>
>>>>
>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>
>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>>
>>>>> Are you planning to add some more fine-grained PM control later?
>>>>
>>>> Yes, I will need to add fine-grained PM control at some point. But, for
>>>> now SoC does not really support low power mode or runtime power saving
>>>> option.
>>>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>
>>>> Just figured out I forgot to add cleanup code in error handling path of
>>>> probe(). Will fix that and send a v4.
>>>
>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>> either, so it's fine for now.
>>>
>>
>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>
> Not of the top of my head, sorry. +CC Matthew, he should know.

I am not an expert at the clock framework nor the power management, but I
did ask around a bit.  No one I asked was planning to change the clk_*()
calls to pm_*() call, but the feedback was that it would be a good idea.

Matthew Gerlach


>
> -- 
> Best regards,
> Marek Vasut
>

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[matthew.gerlach-VuQAYsv1563Yd54FQh9/CA]

On Sun, 24 Sep 2017, Marek Vasut wrote:

> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>
>>
>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>
>>>>
>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>
>>>>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>>>>
>>>>> Are you planning to add some more fine-grained PM control later?
>>>>
>>>> Yes, I will need to add fine-grained PM control at some point. But, for
>>>> now SoC does not really support low power mode or runtime power saving
>>>> option.
>>>> The fact that driver still uses clk_prepare_*() calls to enable/disable
>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>
>>>> Just figured out I forgot to add cleanup code in error handling path of
>>>> probe(). Will fix that and send a v4.
>>>
>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>> either, so it's fine for now.
>>>
>>
>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>
> Not of the top of my head, sorry. +CC Matthew, he should know.

I am not an expert at the clock framework nor the power management, but I
did ask around a bit.  No one I asked was planning to change the clk_*()
calls to pm_*() call, but the feedback was that it would be a good idea.

Matthew Gerlach


>
> -- 
> Best regards,
> Marek Vasut
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/26/2017 12:41 AM, matthew.gerlach@linux.intel.com wrote:
> 
> 
> On Sun, 24 Sep 2017, Marek Vasut wrote:
> 
>> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>>
>>>
>>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>>
>>>>>
>>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>>
>>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>>>
>>>>>> Are you planning to add some more fine-grained PM control later?
>>>>>
>>>>> Yes, I will need to add fine-grained PM control at some point. But,
>>>>> for
>>>>> now SoC does not really support low power mode or runtime power saving
>>>>> option.
>>>>> The fact that driver still uses clk_prepare_*() calls to
>>>>> enable/disable
>>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>>
>>>>> Just figured out I forgot to add cleanup code in error handling
>>>>> path of
>>>>> probe(). Will fix that and send a v4.
>>>>
>>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>>> either, so it's fine for now.
>>>>
>>>
>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>
>> Not of the top of my head, sorry. +CC Matthew, he should know.
> 
> I am not an expert at the clock framework nor the power management, but I
> did ask around a bit.  No one I asked was planning to change the clk_*()
> calls to pm_*() call, but the feedback was that it would be a good idea.

The question is, if we do the replacement, will it break on socfpga ?
A quick test might be useful.

-- 
Best regards,
Marek Vasut

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/26/2017 12:41 AM, matthew.gerlach at linux.intel.com wrote:
> 
> 
> On Sun, 24 Sep 2017, Marek Vasut wrote:
> 
>> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>>
>>>
>>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>>
>>>>>
>>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>>
>>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>>>>>
>>>>>> Are you planning to add some more fine-grained PM control later?
>>>>>
>>>>> Yes, I will need to add fine-grained PM control at some point. But,
>>>>> for
>>>>> now SoC does not really support low power mode or runtime power saving
>>>>> option.
>>>>> The fact that driver still uses clk_prepare_*() calls to
>>>>> enable/disable
>>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>>
>>>>> Just figured out I forgot to add cleanup code in error handling
>>>>> path of
>>>>> probe(). Will fix that and send a v4.
>>>>
>>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>>> either, so it's fine for now.
>>>>
>>>
>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>
>> Not of the top of my head, sorry. +CC Matthew, he should know.
> 
> I am not an expert at the clock framework nor the power management, but I
> did ask around a bit.? No one I asked was planning to change the clk_*()
> calls to pm_*() call, but the feedback was that it would be a good idea.

The question is, if we do the replacement, will it break on socfpga ?
A quick test might be useful.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Marek Vasut]

On 09/26/2017 12:41 AM, matthew.gerlach-VuQAYsv1563Yd54FQh9/CA@public.gmane.org wrote:
> 
> 
> On Sun, 24 Sep 2017, Marek Vasut wrote:
> 
>> On 09/24/2017 03:27 PM, Vignesh R wrote:
>>>
>>>
>>> On 9/24/2017 6:42 PM, Marek Vasut wrote:
>>>> On 09/24/2017 03:08 PM, Vignesh R wrote:
>>>>>
>>>>>
>>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote:
>>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to
>>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe.
>>>>>>>
>>>>>>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>>>>>>
>>>>>> Are you planning to add some more fine-grained PM control later?
>>>>>
>>>>> Yes, I will need to add fine-grained PM control at some point. But,
>>>>> for
>>>>> now SoC does not really support low power mode or runtime power saving
>>>>> option.
>>>>> The fact that driver still uses clk_prepare_*() calls to
>>>>> enable/disable
>>>>> clocks instead of pm_*() calls makes it a bit tricky though.
>>>>>
>>>>> Just figured out I forgot to add cleanup code in error handling
>>>>> path of
>>>>> probe(). Will fix that and send a v4.
>>>>
>>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM
>>>> either, so it's fine for now.
>>>>
>>>
>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>
>> Not of the top of my head, sorry. +CC Matthew, he should know.
> 
> I am not an expert at the clock framework nor the power management, but I
> did ask around a bit.  No one I asked was planning to change the clk_*()
> calls to pm_*() call, but the feedback was that it would be a good idea.

The question is, if we do the replacement, will it break on socfpga ?
A quick test might be useful.

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi Matthew,

On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
[...]
>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>
>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>
>> I am not an expert at the clock framework nor the power management, but I
>> did ask around a bit.  No one I asked was planning to change the clk_*()
>> calls to pm_*() call, but the feedback was that it would be a good idea.
> 
> The question is, if we do the replacement, will it break on socfpga ?
> A quick test might be useful.
> 

yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
like below patch would be helpful:


diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..7ad3e176cc88 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -34,6 +34,7 @@
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
+#include <linux/pm_runtime.h>
 
 #define CQSPI_NAME                     "cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT           16
@@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
                return -ENXIO;
        }
 
-       ret = clk_prepare_enable(cqspi->clk);
-       if (ret) {
-               dev_err(dev, "Cannot enable QSPI clock.\n");
-               return ret;
-       }
+       pm_runtime_enable(dev);
+       pm_runtime_get_sync(dev);
 
        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
 
 



-- 
Regards
Vignesh

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi Matthew,

On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
[...]
>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>
>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>
>> I am not an expert at the clock framework nor the power management, but I
>> did ask around a bit.? No one I asked was planning to change the clk_*()
>> calls to pm_*() call, but the feedback was that it would be a good idea.
> 
> The question is, if we do the replacement, will it break on socfpga ?
> A quick test might be useful.
> 

yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
like below patch would be helpful:


diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..7ad3e176cc88 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -34,6 +34,7 @@
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
+#include <linux/pm_runtime.h>
 
 #define CQSPI_NAME                     "cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT           16
@@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
                return -ENXIO;
        }
 
-       ret = clk_prepare_enable(cqspi->clk);
-       if (ret) {
-               dev_err(dev, "Cannot enable QSPI clock.\n");
-               return ret;
-       }
+       pm_runtime_enable(dev);
+       pm_runtime_get_sync(dev);
 
        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
 
 



-- 
Regards
Vignesh

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi Matthew,

On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
[...]
>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>
>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>
>> I am not an expert at the clock framework nor the power management, but I
>> did ask around a bit.  No one I asked was planning to change the clk_*()
>> calls to pm_*() call, but the feedback was that it would be a good idea.
> 
> The question is, if we do the replacement, will it break on socfpga ?
> A quick test might be useful.
> 

yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
like below patch would be helpful:


diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 53c7d8e0327a..7ad3e176cc88 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -34,6 +34,7 @@
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
 #include <linux/timer.h>
+#include <linux/pm_runtime.h>
 
 #define CQSPI_NAME                     "cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT           16
@@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
                return -ENXIO;
        }
 
-       ret = clk_prepare_enable(cqspi->clk);
-       if (ret) {
-               dev_err(dev, "Cannot enable QSPI clock.\n");
-               return ret;
-       }
+       pm_runtime_enable(dev);
+       pm_runtime_get_sync(dev);
 
        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
 
 



-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[matthew.gerlach]

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


Hi Vignesh,

I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi 
kernel, and it did not go well.  Commands to the flash chip timedout 
resulting in the probe function failing.  I ran into other problems, not 
related to cadence-quadspi, that prevented me from testing against 4.9 and 
4.12 kernels, but I suspect similar behavior.

Matthew Gerlach

On Wed, 27 Sep 2017, Vignesh R wrote:

> Hi Matthew,
>
> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
> [...]
>>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>>
>>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>>
>>> I am not an expert at the clock framework nor the power management, but I
>>> did ask around a bit.  No one I asked was planning to change the clk_*()
>>> calls to pm_*() call, but the feedback was that it would be a good idea.
>>
>> The question is, if we do the replacement, will it break on socfpga ?
>> A quick test might be useful.
>>
>
> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
> like below patch would be helpful:
>
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e0327a..7ad3e176cc88 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -34,6 +34,7 @@
> #include <linux/sched.h>
> #include <linux/spi/spi.h>
> #include <linux/timer.h>
> +#include <linux/pm_runtime.h>
>
> #define CQSPI_NAME                     "cadence-qspi"
> #define CQSPI_MAX_CHIPSELECT           16
> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
>                return -ENXIO;
>        }
>
> -       ret = clk_prepare_enable(cqspi->clk);
> -       if (ret) {
> -               dev_err(dev, "Cannot enable QSPI clock.\n");
> -               return ret;
> -       }
> +       pm_runtime_enable(dev);
> +       pm_runtime_get_sync(dev);
>
>        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>
>
>
>
>
> -- 
> Regards
> Vignesh
>

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[matthew.gerlach at linux.intel.com]

Hi Vignesh,

I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi 
kernel, and it did not go well.  Commands to the flash chip timedout 
resulting in the probe function failing.  I ran into other problems, not 
related to cadence-quadspi, that prevented me from testing against 4.9 and 
4.12 kernels, but I suspect similar behavior.

Matthew Gerlach

On Wed, 27 Sep 2017, Vignesh R wrote:

> Hi Matthew,
>
> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
> [...]
>>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>>
>>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>>
>>> I am not an expert at the clock framework nor the power management, but I
>>> did ask around a bit.? No one I asked was planning to change the clk_*()
>>> calls to pm_*() call, but the feedback was that it would be a good idea.
>>
>> The question is, if we do the replacement, will it break on socfpga ?
>> A quick test might be useful.
>>
>
> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
> like below patch would be helpful:
>
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e0327a..7ad3e176cc88 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -34,6 +34,7 @@
> #include <linux/sched.h>
> #include <linux/spi/spi.h>
> #include <linux/timer.h>
> +#include <linux/pm_runtime.h>
>
> #define CQSPI_NAME                     "cadence-qspi"
> #define CQSPI_MAX_CHIPSELECT           16
> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
>                return -ENXIO;
>        }
>
> -       ret = clk_prepare_enable(cqspi->clk);
> -       if (ret) {
> -               dev_err(dev, "Cannot enable QSPI clock.\n");
> -               return ret;
> -       }
> +       pm_runtime_enable(dev);
> +       pm_runtime_get_sync(dev);
>
>        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>
>
>
>
>
> -- 
> Regards
> Vignesh
>

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi,

On 9/28/2017 8:31 PM, matthew.gerlach@linux.intel.com wrote:
> 
> Hi Vignesh,
> 
> I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi 
> kernel, and it did not go well.  Commands to the flash chip timedout 
> resulting in the probe function failing.  I ran into other problems, not 
> related to cadence-quadspi, that prevented me from testing against 4.9 and 
> 4.12 kernels, but I suspect similar behavior.
> 

Ok, thanks! I will keep the clk_*() calls for now.

Regards
Vignesh
> Matthew Gerlach
> 
> On Wed, 27 Sep 2017, Vignesh R wrote:
> 
>> Hi Matthew,
>>
>> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
>> [...]
>>>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>>>
>>>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>>>
>>>> I am not an expert at the clock framework nor the power management, but I
>>>> did ask around a bit.  No one I asked was planning to change the clk_*()
>>>> calls to pm_*() call, but the feedback was that it would be a good idea.
>>>
>>> The question is, if we do the replacement, will it break on socfpga ?
>>> A quick test might be useful.
>>>
>>
>> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
>> like below patch would be helpful:
>>
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..7ad3e176cc88 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -34,6 +34,7 @@
>> #include <linux/sched.h>
>> #include <linux/spi/spi.h>
>> #include <linux/timer.h>
>> +#include <linux/pm_runtime.h>
>>
>> #define CQSPI_NAME                     "cadence-qspi"
>> #define CQSPI_MAX_CHIPSELECT           16
>> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
>>                return -ENXIO;
>>        }
>>
>> -       ret = clk_prepare_enable(cqspi->clk);
>> -       if (ret) {
>> -               dev_err(dev, "Cannot enable QSPI clock.\n");
>> -               return ret;
>> -       }
>> +       pm_runtime_enable(dev);
>> +       pm_runtime_get_sync(dev);
>>
>>        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>
>>
>>
>>
>>
>> -- 
>> Regards
>> Vignesh
>>
> 

[PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi,

On 9/28/2017 8:31 PM, matthew.gerlach at linux.intel.com wrote:
> 
> Hi Vignesh,
> 
> I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi 
> kernel, and it did not go well.  Commands to the flash chip timedout 
> resulting in the probe function failing.  I ran into other problems, not 
> related to cadence-quadspi, that prevented me from testing against 4.9 and 
> 4.12 kernels, but I suspect similar behavior.
> 

Ok, thanks! I will keep the clk_*() calls for now.

Regards
Vignesh
> Matthew Gerlach
> 
> On Wed, 27 Sep 2017, Vignesh R wrote:
> 
>> Hi Matthew,
>>
>> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
>> [...]
>>>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>>>
>>>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>>>
>>>> I am not an expert at the clock framework nor the power management, but I
>>>> did ask around a bit.? No one I asked was planning to change the clk_*()
>>>> calls to pm_*() call, but the feedback was that it would be a good idea.
>>>
>>> The question is, if we do the replacement, will it break on socfpga ?
>>> A quick test might be useful.
>>>
>>
>> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
>> like below patch would be helpful:
>>
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..7ad3e176cc88 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -34,6 +34,7 @@
>> #include <linux/sched.h>
>> #include <linux/spi/spi.h>
>> #include <linux/timer.h>
>> +#include <linux/pm_runtime.h>
>>
>> #define CQSPI_NAME                     "cadence-qspi"
>> #define CQSPI_MAX_CHIPSELECT           16
>> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
>>                return -ENXIO;
>>        }
>>
>> -       ret = clk_prepare_enable(cqspi->clk);
>> -       if (ret) {
>> -               dev_err(dev, "Cannot enable QSPI clock.\n");
>> -               return ret;
>> -       }
>> +       pm_runtime_enable(dev);
>> +       pm_runtime_get_sync(dev);
>>
>>        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>
>>
>>
>>
>>
>> -- 
>> Regards
>> Vignesh
>>
> 

Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support

[Vignesh R]

Hi,

On 9/28/2017 8:31 PM, matthew.gerlach-VuQAYsv1563Yd54FQh9/CA@public.gmane.org wrote:
> 
> Hi Vignesh,
> 
> I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi 
> kernel, and it did not go well.  Commands to the flash chip timedout 
> resulting in the probe function failing.  I ran into other problems, not 
> related to cadence-quadspi, that prevented me from testing against 4.9 and 
> 4.12 kernels, but I suspect similar behavior.
> 

Ok, thanks! I will keep the clk_*() calls for now.

Regards
Vignesh
> Matthew Gerlach
> 
> On Wed, 27 Sep 2017, Vignesh R wrote:
> 
>> Hi Matthew,
>>
>> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote:
>> [...]
>>>>>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for
>>>>>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see
>>>>>> if its possible to get rid of clk_*() calls in favor of pm_*() calls.
>>>>>
>>>>> Not of the top of my head, sorry. +CC Matthew, he should know.
>>>>
>>>> I am not an expert at the clock framework nor the power management, but I
>>>> did ask around a bit.  No one I asked was planning to change the clk_*()
>>>> calls to pm_*() call, but the feedback was that it would be a good idea.
>>>
>>> The question is, if we do the replacement, will it break on socfpga ?
>>> A quick test might be useful.
>>>
>>
>> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls
>> like below patch would be helpful:
>>
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 53c7d8e0327a..7ad3e176cc88 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -34,6 +34,7 @@
>> #include <linux/sched.h>
>> #include <linux/spi/spi.h>
>> #include <linux/timer.h>
>> +#include <linux/pm_runtime.h>
>>
>> #define CQSPI_NAME                     "cadence-qspi"
>> #define CQSPI_MAX_CHIPSELECT           16
>> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev)
>>                return -ENXIO;
>>        }
>>
>> -       ret = clk_prepare_enable(cqspi->clk);
>> -       if (ret) {
>> -               dev_err(dev, "Cannot enable QSPI clock.\n");
>> -               return ret;
>> -       }
>> +       pm_runtime_enable(dev);
>> +       pm_runtime_get_sync(dev);
>>
>>        cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>
>>
>>
>>
>>
>> -- 
>> Regards
>> Vignesh
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html