linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi
@ 2019-08-27  3:58 Ramuthevar, Vadivel MuruganX
  2019-08-27  3:58 ` [PATCH v2 1/3] " Ramuthevar, Vadivel MuruganX
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-08-27  3:58 UTC (permalink / raw)
  To: linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, Ramuthevar, Vadivel MuruganX, miquel.raynal,
	jwboyer, computersforpeace, dwmw2, cyrille.pitchen,
	andriy.shevchenko

mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM
mtd: spi-nor: cadence-quadspi: disable the auto-poll for Intel LGM

changes from V1:
  - many thanks to Vignesh for review comments.
  - split 2 patches one for DMA and DAC and other one
    is disable the auto-poll
  - removed ahb_phy_addr and used existing trigger_address
  - trigger_address property used.
  
Ramuthevar Vadivel Murugan (3):
  dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi
  mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM
  mtd: spi-nor: cadence-quadspi: disable the auto-poll for Intel LGM

 .../devicetree/bindings/mtd/cadence-quadspi.txt    |  1 +
 drivers/mtd/spi-nor/Kconfig                        |  2 +-
 drivers/mtd/spi-nor/cadence-quadspi.c              | 45 ++++++++++++++++++++--
 3 files changed, 44 insertions(+), 4 deletions(-)

-- 
2.11.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi
  2019-08-27  3:58 [PATCH v2 0/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi Ramuthevar, Vadivel MuruganX
@ 2019-08-27  3:58 ` Ramuthevar, Vadivel MuruganX
  2019-09-02 13:39   ` Rob Herring
  2019-08-27  3:58 ` [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM Ramuthevar, Vadivel MuruganX
  2019-08-27  3:58 ` [PATCH v2 3/3] mtd: spi-nor: cadence-quadspi: disable the auto-poll " Ramuthevar, Vadivel MuruganX
  2 siblings, 1 reply; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-08-27  3:58 UTC (permalink / raw)
  To: linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, Ramuthevar Vadivel Murugan, miquel.raynal,
	jwboyer, computersforpeace, dwmw2, cyrille.pitchen,
	andriy.shevchenko

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Add new vendor specific compatible string to check Intel's Lightning
Mountain(LGM) QSPI features enablement in cadence-quadspi driver.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index 945be7d5b236..8ace832a2d80 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -5,6 +5,7 @@ Required properties:
 	Generic default - "cdns,qspi-nor".
 	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
 	For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
+	For Intel LGM SoC - "intel,lgm-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.11.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM
  2019-08-27  3:58 [PATCH v2 0/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi Ramuthevar, Vadivel MuruganX
  2019-08-27  3:58 ` [PATCH v2 1/3] " Ramuthevar, Vadivel MuruganX
@ 2019-08-27  3:58 ` Ramuthevar, Vadivel MuruganX
  2019-09-09  6:05   ` Vignesh Raghavendra
  2019-08-27  3:58 ` [PATCH v2 3/3] mtd: spi-nor: cadence-quadspi: disable the auto-poll " Ramuthevar, Vadivel MuruganX
  2 siblings, 1 reply; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-08-27  3:58 UTC (permalink / raw)
  To: linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, Ramuthevar Vadivel Murugan, miquel.raynal,
	jwboyer, computersforpeace, dwmw2, cyrille.pitchen,
	andriy.shevchenko

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

on Intel's Lightning Mountain(LGM) SoCs QSPI controller do not use
Direct Memory Access(DMA) and Direct Access Controller(DAC).

This patch introduces to properly disable the DMA and DAC
for data transfer instead it uses indirect data transfer.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
---
 drivers/mtd/spi-nor/Kconfig           |  2 +-
 drivers/mtd/spi-nor/cadence-quadspi.c | 21 ++++++++++++++++++---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 6de83277ce8b..ba2e372ae514 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -34,7 +34,7 @@ config SPI_ASPEED_SMC
 
 config SPI_CADENCE_QUADSPI
 	tristate "Cadence Quad SPI controller"
-	depends on OF && (ARM || ARM64 || COMPILE_TEST)
+	depends on OF && (ARM || ARM64 || COMPILE_TEST || X86)
 	help
 	  Enable support for the Cadence Quad SPI Flash controller.
 
diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 67f15a1f16fd..69fa13e95110 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -517,12 +517,16 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 	void __iomem *ahb_base = cqspi->ahb_base;
+	u32 trigger_address = cqspi->trigger_address;
 	unsigned int remaining = n_rx;
 	unsigned int mod_bytes = n_rx % 4;
 	unsigned int bytes_to_read = 0;
 	u8 *rxbuf_end = rxbuf + n_rx;
 	int ret = 0;
 
+	if (!f_pdata->use_direct_mode)
+		writel(trigger_address, reg_base + CQSPI_REG_INDIRECTTRIGGER);
+
 	writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
 	writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
 
@@ -609,6 +613,14 @@ static int cqspi_write_setup(struct spi_nor *nor)
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 
+	/* Disable the DMA and direct access controller */
+	if (!f_pdata->use_direct_mode) {
+		reg = readl(reg_base + CQSPI_REG_CONFIG);
+		reg &= ~CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
+		reg &= ~CQSPI_REG_CONFIG_DMA_MASK;
+		writel(reg, reg_base + CQSPI_REG_CONFIG);
+	}
+
 	/* Set opcode. */
 	reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
 	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
@@ -1171,7 +1183,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
+	if (!of_device_is_compatible(np, "intel,lgm-qspi"))
+		cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
 
 	return 0;
 }
@@ -1301,7 +1314,8 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 		f_pdata->registered = true;
 
 		if (mtd->size <= cqspi->ahb_size) {
-			f_pdata->use_direct_mode = true;
+			f_pdata->use_direct_mode =
+				!(of_device_is_compatible(np, "intel,lgm-qspi"));
 			dev_dbg(nor->dev, "using direct mode for %s\n",
 				mtd->name);
 
@@ -1347,7 +1361,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	/* Obtain QSPI clock. */
-	cqspi->clk = devm_clk_get(dev, NULL);
+	cqspi->clk = devm_clk_get(dev, "qspi");
 	if (IS_ERR(cqspi->clk)) {
 		dev_err(dev, "Cannot claim QSPI clock.\n");
 		return PTR_ERR(cqspi->clk);
@@ -1369,6 +1383,7 @@ static int cqspi_probe(struct platform_device *pdev)
 		return PTR_ERR(cqspi->ahb_base);
 	}
 	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
+	cqspi->trigger_address = res_ahb->start;
 	cqspi->ahb_size = resource_size(res_ahb);
 
 	init_completion(&cqspi->transfer_complete);
-- 
2.11.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] mtd: spi-nor: cadence-quadspi: disable the auto-poll for Intel LGM
  2019-08-27  3:58 [PATCH v2 0/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi Ramuthevar, Vadivel MuruganX
  2019-08-27  3:58 ` [PATCH v2 1/3] " Ramuthevar, Vadivel MuruganX
  2019-08-27  3:58 ` [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM Ramuthevar, Vadivel MuruganX
@ 2019-08-27  3:58 ` Ramuthevar, Vadivel MuruganX
  2 siblings, 0 replies; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-08-27  3:58 UTC (permalink / raw)
  To: linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, Ramuthevar Vadivel Murugan, miquel.raynal,
	jwboyer, computersforpeace, dwmw2, cyrille.pitchen,
	andriy.shevchenko

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

On Intel's Lightning Mountain(LGM) SoC QSPI controller do not auto-poll.
This patch introduces to properly disable the auto-polling feature to
improve the performance of cadence-quadspi.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 69fa13e95110..94aa40e868a1 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -134,6 +134,8 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_RD_INSTR_TYPE_DATA_MASK	0x3
 #define CQSPI_REG_RD_INSTR_DUMMY_MASK		0x1F
 
+#define CQSPI_REG_WR_COMPLETION_CTRL		0x38
+#define CQSPI_REG_WR_COMPLETION_DISABLE_AUTO_POLL	BIT(14)
 #define CQSPI_REG_WR_INSTR			0x08
 #define CQSPI_REG_WR_INSTR_OPCODE_LSB		0
 #define CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB	12
@@ -470,6 +472,18 @@ static int cqspi_command_write_addr(struct spi_nor *nor,
 	return cqspi_exec_flash_cmd(cqspi, reg);
 }
 
+static int cqspi_disable_auto_poll(struct cqspi_st *cqspi)
+{
+	void __iomem *reg_base = cqspi->iobase;
+	unsigned int reg;
+
+	reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+	reg |= CQSPI_REG_WR_COMPLETION_DISABLE_AUTO_POLL;
+	writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+
+	return 0;
+}
+
 static int cqspi_read_setup(struct spi_nor *nor)
 {
 	struct cqspi_flash_pdata *f_pdata = nor->priv;
@@ -507,6 +521,11 @@ static int cqspi_read_setup(struct spi_nor *nor)
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
 	reg |= (nor->addr_width - 1);
 	writel(reg, reg_base + CQSPI_REG_SIZE);
+
+	/* Disable auto-polling */
+	if (!f_pdata->use_direct_mode)
+		cqspi_disable_auto_poll(cqspi);
+
 	return 0;
 }
 
@@ -631,6 +650,11 @@ static int cqspi_write_setup(struct spi_nor *nor)
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
 	reg |= (nor->addr_width - 1);
 	writel(reg, reg_base + CQSPI_REG_SIZE);
+
+	/* Disable auto-polling */
+	if (!f_pdata->use_direct_mode)
+		cqspi_disable_auto_poll(cqspi);
+
 	return 0;
 }
 
-- 
2.11.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi
  2019-08-27  3:58 ` [PATCH v2 1/3] " Ramuthevar, Vadivel MuruganX
@ 2019-09-02 13:39   ` Rob Herring
  2019-09-03  2:03     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2019-09-02 13:39 UTC (permalink / raw)
  To: Ramuthevar, Vadivel MuruganX
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, Ramuthevar Vadivel Murugan, linux-mtd,
	miquel.raynal, jwboyer, computersforpeace, dwmw2,
	cyrille.pitchen, andriy.shevchenko

On Tue, 27 Aug 2019 11:58:25 +0800, "Ramuthevar,Vadivel MuruganX"          wrote:
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> 
> Add new vendor specific compatible string to check Intel's Lightning
> Mountain(LGM) QSPI features enablement in cadence-quadspi driver.
> 
> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi
  2019-09-02 13:39   ` Rob Herring
@ 2019-09-03  2:03     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 0 replies; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-09-03  2:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, vigneshr,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, linux-mtd, miquel.raynal, jwboyer,
	computersforpeace, dwmw2, cyrille.pitchen, andriy.shevchenko

Hi Rob,

  Thank you for the review and Acked-by.

On 2/9/2019 9:39 PM, Rob Herring wrote:
> On Tue, 27 Aug 2019 11:58:25 +0800, "Ramuthevar,Vadivel MuruganX"          wrote:
>> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>>
>> Add new vendor specific compatible string to check Intel's Lightning
>> Mountain(LGM) QSPI features enablement in cadence-quadspi driver.
>>
>> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>> ---
>>   Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
>>   1 file changed, 1 insertion(+)
>>
> Acked-by: Rob Herring <robh@kernel.org>

Acked-by tag will be updated in next patch-set.

Best Regards
Vadivel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM
  2019-08-27  3:58 ` [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM Ramuthevar, Vadivel MuruganX
@ 2019-09-09  6:05   ` Vignesh Raghavendra
  2019-09-09  7:32     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 1 reply; 8+ messages in thread
From: Vignesh Raghavendra @ 2019-09-09  6:05 UTC (permalink / raw)
  To: Ramuthevar,Vadivel MuruganX, linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, andriy.shevchenko,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, miquel.raynal, jwboyer, computersforpeace,
	dwmw2, cyrille.pitchen

Hi,

On 27/08/19 9:28 AM, Ramuthevar,Vadivel MuruganX wrote:
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> 
> on Intel's Lightning Mountain(LGM) SoCs QSPI controller do not use
> Direct Memory Access(DMA) and Direct Access Controller(DAC).
> 
> This patch introduces to properly disable the DMA and DAC
> for data transfer instead it uses indirect data transfer.
> 
> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> ---
>  drivers/mtd/spi-nor/Kconfig           |  2 +-
>  drivers/mtd/spi-nor/cadence-quadspi.c | 21 ++++++++++++++++++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 6de83277ce8b..ba2e372ae514 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -34,7 +34,7 @@ config SPI_ASPEED_SMC
>  
>  config SPI_CADENCE_QUADSPI
>  	tristate "Cadence Quad SPI controller"
> -	depends on OF && (ARM || ARM64 || COMPILE_TEST)
> +	depends on OF && (ARM || ARM64 || COMPILE_TEST || X86)
>  	help
>  	  Enable support for the Cadence Quad SPI Flash controller.
>  
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 67f15a1f16fd..69fa13e95110 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -517,12 +517,16 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
>  	void __iomem *ahb_base = cqspi->ahb_base;
> +	u32 trigger_address = cqspi->trigger_address;
>  	unsigned int remaining = n_rx;
>  	unsigned int mod_bytes = n_rx % 4;
>  	unsigned int bytes_to_read = 0;
>  	u8 *rxbuf_end = rxbuf + n_rx;
>  	int ret = 0;
>  
> +	if (!f_pdata->use_direct_mode)
> +		writel(trigger_address, reg_base + CQSPI_REG_INDIRECTTRIGGER);
> +

Again, as I pointed out previously, this should not be needed.
cqspi_controller_init() already does above configuration and no need to
touch this register on every read.

>  	writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
>  	writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
>  
> @@ -609,6 +613,14 @@ static int cqspi_write_setup(struct spi_nor *nor)
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
>  
> +	/* Disable the DMA and direct access controller */
> +	if (!f_pdata->use_direct_mode) {
> +		reg = readl(reg_base + CQSPI_REG_CONFIG);
> +		reg &= ~CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
> +		reg &= ~CQSPI_REG_CONFIG_DMA_MASK;
> +		writel(reg, reg_base + CQSPI_REG_CONFIG);
> +	}
> +

You did not respond to my previous comment. Why would you need to clear
CQSPI_REG_CONFIG_DMA_MASK field, if reset default is 0 anyway?

>  	/* Set opcode. */
>  	reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
>  	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
> @@ -1171,7 +1183,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
>  		return -ENXIO;
>  	}
>  
> -	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
> +	if (!of_device_is_compatible(np, "intel,lgm-qspi"))
> +		cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
>  

If you don't want to use this property, then just drop it from your DT.
Don't override it in the driver based on compatible.

>  	return 0;
>  }
> @@ -1301,7 +1314,8 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  		f_pdata->registered = true;
>  
>  		if (mtd->size <= cqspi->ahb_size) {
> -			f_pdata->use_direct_mode = true;
> +			f_pdata->use_direct_mode =
> +				!(of_device_is_compatible(np, "intel,lgm-qspi"));

Looks like, you haven't followed any of my advice. Please add a quirk
flag to disable DAC mode. Something like:

#define CQSPI_DISABLE_DAC_MODE BIT(1)

static const struct cqspi_driver_platdata intel_lgm_qspi = {
        .hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
        .quirks = CQSPI_DISABLE_DAC_MODE,
};

static const struct of_device_id cqspi_dt_ids[] = {

	...

        {
                .compatible = "intel,lgm-qspi",
                .data = &intel_lgm_qspi,
        },

	...
};


Then in cqspi_setup_flash(),

	if (mtd->size <= cqspi->ahb_size &&
		!ddata->quirks & CQSPI_DISABLE_DAC_MODE) {
		f_pdata->use_direct_mode = true;
		...
	}	


>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>  				mtd->name);
>  
> @@ -1347,7 +1361,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Obtain QSPI clock. */
> -	cqspi->clk = devm_clk_get(dev, NULL);
> +	cqspi->clk = devm_clk_get(dev, "qspi");

This will break DT backward compatibility. Existing DTs don't have
clock-names = "qspi". Hence, this code will error out.
What I meant in the previous mail was: if device does not have multiple
clk inputs then there is no need for clock-names and there is no need to
touch this part of code.

	cqspi->clk = devm_clk_get(dev, NULL);

This should just work fine even on your board. So drop this hunk.

>  	if (IS_ERR(cqspi->clk)) {
>  		dev_err(dev, "Cannot claim QSPI clock.\n");
>  		return PTR_ERR(cqspi->clk);
> @@ -1369,6 +1383,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  		return PTR_ERR(cqspi->ahb_base);
>  	}
>  	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
> +	cqspi->trigger_address = res_ahb->start;

Nope, this is not needed. See:
https://elixir.bootlin.com/linux/v5.3-rc6/source/drivers/mtd/spi-nor/cadence-quadspi.c#L1168

Populate the trigger-address using cdns,trigger-address property in DT

>  	cqspi->ahb_size = resource_size(res_ahb);
>  
>  	init_completion(&cqspi->transfer_complete);
> 

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM
  2019-09-09  6:05   ` Vignesh Raghavendra
@ 2019-09-09  7:32     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 0 replies; 8+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2019-09-09  7:32 UTC (permalink / raw)
  To: Vignesh Raghavendra, linux-mtd
  Cc: cheol.yong.kim, devicetree, tudor.ambarus, andriy.shevchenko,
	boris.brezillon, richard, qi-ming.wu, linux-kernel,
	david.oberhollenzer, miquel.raynal, jwboyer, computersforpeace,
	dwmw2, cyrille.pitchen

Hi Vignesh,

Thank you so much for the review comments and your time.

On 9/9/2019 2:05 PM, Vignesh Raghavendra wrote:
> Hi,
>
> On 27/08/19 9:28 AM, Ramuthevar,Vadivel MuruganX wrote:
>> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>>
>> on Intel's Lightning Mountain(LGM) SoCs QSPI controller do not use
>> Direct Memory Access(DMA) and Direct Access Controller(DAC).
>>
>> This patch introduces to properly disable the DMA and DAC
>> for data transfer instead it uses indirect data transfer.
>>
>> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>> ---
>>   drivers/mtd/spi-nor/Kconfig           |  2 +-
>>   drivers/mtd/spi-nor/cadence-quadspi.c | 21 ++++++++++++++++++---
>>   2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 6de83277ce8b..ba2e372ae514 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -34,7 +34,7 @@ config SPI_ASPEED_SMC
>>   
>>   config SPI_CADENCE_QUADSPI
>>   	tristate "Cadence Quad SPI controller"
>> -	depends on OF && (ARM || ARM64 || COMPILE_TEST)
>> +	depends on OF && (ARM || ARM64 || COMPILE_TEST || X86)
>>   	help
>>   	  Enable support for the Cadence Quad SPI Flash controller.
>>   
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 67f15a1f16fd..69fa13e95110 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -517,12 +517,16 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
>>   	struct cqspi_st *cqspi = f_pdata->cqspi;
>>   	void __iomem *reg_base = cqspi->iobase;
>>   	void __iomem *ahb_base = cqspi->ahb_base;
>> +	u32 trigger_address = cqspi->trigger_address;
>>   	unsigned int remaining = n_rx;
>>   	unsigned int mod_bytes = n_rx % 4;
>>   	unsigned int bytes_to_read = 0;
>>   	u8 *rxbuf_end = rxbuf + n_rx;
>>   	int ret = 0;
>>   
>> +	if (!f_pdata->use_direct_mode)
>> +		writel(trigger_address, reg_base + CQSPI_REG_INDIRECTTRIGGER);
>> +
> Again, as I pointed out previously, this should not be needed.
> cqspi_controller_init() already does above configuration and no need to
> touch this register on every read.
Agreed!, drop this one.
>>   	writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
>>   	writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
>>   
>> @@ -609,6 +613,14 @@ static int cqspi_write_setup(struct spi_nor *nor)
>>   	struct cqspi_st *cqspi = f_pdata->cqspi;
>>   	void __iomem *reg_base = cqspi->iobase;
>>   
>> +	/* Disable the DMA and direct access controller */
>> +	if (!f_pdata->use_direct_mode) {
>> +		reg = readl(reg_base + CQSPI_REG_CONFIG);
>> +		reg &= ~CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
>> +		reg &= ~CQSPI_REG_CONFIG_DMA_MASK;
>> +		writel(reg, reg_base + CQSPI_REG_CONFIG);
>> +	}
>> +
> You did not respond to my previous comment. Why would you need to clear
> CQSPI_REG_CONFIG_DMA_MASK field, if reset default is 0 anyway?
while testing on the real setup it is not working for me, double confirm 
and drop it.
>>   	/* Set opcode. */
>>   	reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
>>   	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
>> @@ -1171,7 +1183,8 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
>>   		return -ENXIO;
>>   	}
>>   
>> -	cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
>> +	if (!of_device_is_compatible(np, "intel,lgm-qspi"))
>> +		cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en");
>>   
> If you don't want to use this property, then just drop it from your DT.
> Don't override it in the driver based on compatible.
Sure.
>>   	return 0;
>>   }
>> @@ -1301,7 +1314,8 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>   		f_pdata->registered = true;
>>   
>>   		if (mtd->size <= cqspi->ahb_size) {
>> -			f_pdata->use_direct_mode = true;
>> +			f_pdata->use_direct_mode =
>> +				!(of_device_is_compatible(np, "intel,lgm-qspi"));
> Looks like, you haven't followed any of my advice. Please add a quirk
> flag to disable DAC mode. Something like:

Sorry,  on real setup kernel got crash if we add quirks, so that is the 
reason I started using DT compatible to

avoid crashing and also don't want disturb the existing functionalities.

Let me check once again.

Thank you so much for your valuable comments.

> #define CQSPI_DISABLE_DAC_MODE BIT(1)
>
> static const struct cqspi_driver_platdata intel_lgm_qspi = {
>          .hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
>          .quirks = CQSPI_DISABLE_DAC_MODE,
> };
>
> static const struct of_device_id cqspi_dt_ids[] = {
>
> 	...
>
>          {
>                  .compatible = "intel,lgm-qspi",
>                  .data = &intel_lgm_qspi,
>          },
>
> 	...
> };
>
>
> Then in cqspi_setup_flash(),
>
> 	if (mtd->size <= cqspi->ahb_size &&
> 		!ddata->quirks & CQSPI_DISABLE_DAC_MODE) {
> 		f_pdata->use_direct_mode = true;
> 		...
> 	}	
>
>
>>   			dev_dbg(nor->dev, "using direct mode for %s\n",
>>   				mtd->name);
>>   
>> @@ -1347,7 +1361,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	/* Obtain QSPI clock. */
>> -	cqspi->clk = devm_clk_get(dev, NULL);
>> +	cqspi->clk = devm_clk_get(dev, "qspi");
> This will break DT backward compatibility. Existing DTs don't have
> clock-names = "qspi". Hence, this code will error out.
> What I meant in the previous mail was: if device does not have multiple
> clk inputs then there is no need for clock-names and there is no need to
> touch this part of code.
>
> 	cqspi->clk = devm_clk_get(dev, NULL);
>
> This should just work fine even on your board. So drop this hunk.
Sure, I will drop it.
>>   	if (IS_ERR(cqspi->clk)) {
>>   		dev_err(dev, "Cannot claim QSPI clock.\n");
>>   		return PTR_ERR(cqspi->clk);
>> @@ -1369,6 +1383,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>   		return PTR_ERR(cqspi->ahb_base);
>>   	}
>>   	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
>> +	cqspi->trigger_address = res_ahb->start;
> Nope, this is not needed. See:
> https://elixir.bootlin.com/linux/v5.3-rc6/source/drivers/mtd/spi-nor/cadence-quadspi.c#L1168
>
> Populate the trigger-address using cdns,trigger-address property in DT

Agreed!,  fix it in the next version.

Best Regards
vadivel
>>   	cqspi->ahb_size = resource_size(res_ahb);
>>   
>>   	init_completion(&cqspi->transfer_complete);
>>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-09-09  7:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27  3:58 [PATCH v2 0/3] dt-bindings: mtd: cadence-qspi:add support for Intel lgm-qspi Ramuthevar, Vadivel MuruganX
2019-08-27  3:58 ` [PATCH v2 1/3] " Ramuthevar, Vadivel MuruganX
2019-09-02 13:39   ` Rob Herring
2019-09-03  2:03     ` Ramuthevar, Vadivel MuruganX
2019-08-27  3:58 ` [PATCH v2 2/3] mtd: spi-nor: cadence-quadspi: disable DMA and DAC for Intel LGM Ramuthevar, Vadivel MuruganX
2019-09-09  6:05   ` Vignesh Raghavendra
2019-09-09  7:32     ` Ramuthevar, Vadivel MuruganX
2019-08-27  3:58 ` [PATCH v2 3/3] mtd: spi-nor: cadence-quadspi: disable the auto-poll " Ramuthevar, Vadivel MuruganX

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).