Skip to contentSkip to conversation

spi: cadence_qspi: Fix the indirect ahb trigger address setting

PATCH New Vikas Manocha 3 participants 6 messages May 29, 2015 – Jun 2, 2015 SPI

lore
Share & tools
lore

Vikas Manocha

author
#0 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by fetching/storing data from/to the Controllers SRAM.

Signed-off-by Vikas Manocha <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/spi/cadence_qspi_apb.c +2 -5
index 00a115f..855e5c7 100644
@@ -50,7 +50,6 @@
 #define CQSPI_INST_TYPE_QUAD			(2)
 
 #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
 #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
 #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
 		addr_bytes = cmdlen - 1;
 
 	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Configure SRAM partition for read. */
 	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase +

@@ -798,8 +796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 		return -EINVAL;
 	}
 	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	writel(CQSPI_REG_SRAM_PARTITION_WR,
 	       plat->regbase + CQSPI_REG_SRAMPARTITION);
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Vikas,

(added Dinh, Graham and Marek to Cc)

Quoted 37 lines · Vikas Manocha

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by fetching/storing data from/to the Controllers SRAM.

Signed-off-by Vikas Manocha <[email protected]>
---
  drivers/spi/cadence_qspi_apb.c |    7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)
drivers/spi/cadence_qspi_apb.c +1 -3
index 00a115f..855e5c7 100644
@@ -50,7 +50,6 @@
  #define CQSPI_INST_TYPE_QUAD			(2)
 
  #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
  #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
  		addr_bytes = cmdlen - 1;
 
  	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
  	/* Configure SRAM partition for read. */
  	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase +

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, · line 799

Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +1 -2
@@ -798,8 +796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
  		return -EINVAL;
  	}
  	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Might I ask on which SoC you are using this driver? Which problem are you experiencing exactly?

Graham, can you please comment on this patch?

Thanks,
Stefan

author
#2 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Stefan,

Quoted 39 lines · Stefan Roese

Hi Vikas,

(added Dinh, Graham and Marek to Cc)

Quoted 34 lines · Vikas Manocha

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by fetching/storing data from/to the Controllers SRAM.

Signed-off-by Vikas Manocha <[email protected]>
---
  drivers/spi/cadence_qspi_apb.c |    7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)
drivers/spi/cadence_qspi_apb.c +1 -3
index 00a115f..855e5c7 100644
@@ -50,7 +50,6 @@
  #define CQSPI_INST_TYPE_QUAD			(2)
 
  #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
  #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
  		addr_bytes = cmdlen - 1;
 
  	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
Quoted 11 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, · line 799

Quoted 9 lines
drivers/spi/cadence_qspi_apb.c +1 -2
  	/* Configure SRAM partition for read. */
  	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase +

@@ -798,8 +796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
  		return -EINVAL;
  	}
  	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Might I ask on which SoC you are using this driver? Which problem are you experiencing exactly?

I am using ST's soc stv0991. The spi flash memory is mapped at address 0x4000_0000. By Masking with 0xF_FFFF, the value to be loaded in the indirect trigger register becomes zero & then spi read/write does not work. By removing or changing the mask to 0xFFFF_FFFF, the loaded value in indirect trigger register is 0x4000_0000 & spi read/write works fine.

It should be same of socfpga also after masking with 0xF_FFFF, wondering how it works there.

No masking is done in the driver while polling fifo for read/write, plat->ahbbase was used directly. Any idea why this masking was done for ahb trigger base register?

Rgds,
Vikas

Quoted 8 lines

Graham, can you please comment on this patch?

Thanks,
Stefan

.

#3 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Vikas,

Quoted 52 lines · vikasm
Quoted 39 lines · Stefan Roese

Hi Vikas,

(added Dinh, Graham and Marek to Cc)

Quoted 34 lines · Vikas Manocha

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by fetching/storing data from/to the Controllers SRAM.

Signed-off-by Vikas Manocha <[email protected]>
---
   drivers/spi/cadence_qspi_apb.c |    7 ++-----
   1 file changed, 2 insertions(+), 5 deletions(-)
drivers/spi/cadence_qspi_apb.c +1 -3
index 00a115f..855e5c7 100644
@@ -50,7 +50,6 @@
   #define CQSPI_INST_TYPE_QUAD			(2)
 
   #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
   #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
   #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
   		addr_bytes = cmdlen - 1;
 
   	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
Quoted 11 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, · line 799

Quoted 9 lines
drivers/spi/cadence_qspi_apb.c +1 -2
   	/* Configure SRAM partition for read. */
   	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase +

@@ -798,8 +796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
   		return -EINVAL;
   	}
   	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Might I ask on which SoC you are using this driver? Which problem are you experiencing exactly?

Quoted 8 lines

I am using ST's soc stv0991. The spi flash memory is mapped at address 0x4000_0000. By Masking with 0xF_FFFF, the value to be loaded in the indirect trigger register becomes zero & then spi read/write does not work. By removing or changing the mask to 0xFFFF_FFFF, the loaded value in indirect trigger register is 0x4000_0000 & spi read/write works fine.

It should be same of socfpga also after masking with 0xF_FFFF, wondering how it works there.

Yes, I'm wondering a bit myself. Thats why I added Graham to Cc as he is the author of the original driver. And knows this far better than I do.

Quoted 3 lines

No masking is done in the driver while polling fifo for read/write, plat->ahbbase was used directly. Any idea why this masking was done for ahb trigger base register?

Graham? Do you have any insight here please?

Thanks,
Stefan

author
#4 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Graham,

Quoted 80 lines

-----Original Message-----
From: Stefan Roese [mailto:sr at denx.de]
Sent: Monday, June 01, 2015 4:34 AM
To: Vikas MANOCHA; u-boot at lists.denx.de; Graham Moore
Cc: Dinh Nguyen; Marek Vasut
Subject: Re: [PATCH] spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Vikas,

Quoted 6 lines · vikasm
Quoted 5 lines · Stefan Roese

Hi Vikas,

(added Dinh, Graham and Marek to Cc)

On 29.05.2015 03:22, Vikas Manocha wrote:

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by

fetching/storing data from/to the Controllers SRAM.

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Signed-off-by Vikas Manocha <[email protected]>
---
   drivers/spi/cadence_qspi_apb.c |    7 ++-----
   1 file changed, 2 insertions(+), 5 deletions(-)
(quoted diff) +0 -0

b/drivers/spi/cadence_qspi_apb.c index 00a115f..855e5c7 100644

drivers/spi/cadence_qspi_apb.c +0 -0
@@ -50,7 +50,6 @@
   #define CQSPI_INST_TYPE_QUAD			(2)
Quoted 6 lines
Quoted 6 lines

drivers/spi/cadence_qspi_apb.c int

Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +0 -1
   #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
   #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
   #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int

cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 4 lines
Quoted 4 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 4 lines
drivers/spi/cadence_qspi_apb.c +0 -1
   		addr_bytes = cmdlen - 1;
 
   	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase &

CQSPI_INDIRECTTRIGGER_ADDR_MASK),

Quoted 6 lines
Quoted 6 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +2 -1
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase +
+CQSPI_REG_INDIRECTTRIGGER);
 
   	/* Configure SRAM partition for read. */
   	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase + @@ -

798,8

Quoted 1 line
Quoted 1 line
Quoted 1 line

+796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct

cadence_spi_platdata *plat,

Quoted 4 lines
Quoted 4 lines

drivers/spi/cadence_qspi_apb.c

Quoted 4 lines
   		return -EINVAL;
   	}
   	/* Setup the indirect trigger address */
drivers/spi/cadence_qspi_apb.c +0 -1
-	writel(((u32)plat->ahbbase &

CQSPI_INDIRECTTRIGGER_ADDR_MASK),

Quoted 6 lines
Quoted 5 lines

drivers/spi/cadence_qspi_apb.c

Quoted 3 lines
drivers/spi/cadence_qspi_apb.c +2 -1
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase +
+CQSPI_REG_INDIRECTTRIGGER);

Might I ask on which SoC you are using this driver? Which problem are you experiencing exactly?

Quoted 8 lines

I am using ST's soc stv0991. The spi flash memory is mapped at address 0x4000_0000. By Masking with 0xF_FFFF, the value to be loaded in the indirect trigger register becomes zero & then spi read/write does not work. By removing or changing the mask to 0xFFFF_FFFF, the loaded value in indirect trigger register is 0x4000_0000 & spi read/write works fine.

It should be same of socfpga also after masking with 0xF_FFFF, wondering how it works there.

Quoted 8 lines

Yes, I'm wondering a bit myself. Thats why I added Graham to Cc as he is the author of the original driver. And knows this far better than I do.

Quoted 3 lines

No masking is done in the driver while polling fifo for read/write, plat->ahbbase was used directly. Any idea why this masking was done for ahb trigger base register?

Graham? Do you have any insight here please?

Any idea ?

Rgds,
Vikas

Quoted 3 lines

Thanks,
Stefan

#5 lore raw

PATCH 1/1 spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hello,

OK, first of all, I'm not the original author, just the one who has to fix it nowadays. There are many things that have confused me in this driver.

The mask issue seems familiar, let me look at it, and I'll get back to you.

-Graham

Quoted 100 lines · Vikas MANOCHA

Hi Graham,

Quoted 80 lines

-----Original Message-----
From: Stefan Roese [mailto:sr at denx.de]
Sent: Monday, June 01, 2015 4:34 AM
To: Vikas MANOCHA; u-boot at lists.denx.de; Graham Moore
Cc: Dinh Nguyen; Marek Vasut
Subject: Re: [PATCH] spi: cadence_qspi: Fix the indirect ahb trigger address setting

Hi Vikas,

Quoted 6 lines · vikasm
Quoted 5 lines · Stefan Roese

Hi Vikas,

(added Dinh, Graham and Marek to Cc)

On 29.05.2015 03:22, Vikas Manocha wrote:

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Trigger base address can be set to the spi flash address without any masking, here is the explanation of the register.

QSPI_IND_AHB_ADDR_TRIGGER :
Trigger Address is the base address that is used by the AHB controller for indirect accesses. When the incoming AHB access address matches a range of addresses from this trigger address to the trigger address + 15, then the AHB request is completed by

fetching/storing data from/to the Controllers SRAM.

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Signed-off-by Vikas Manocha <[email protected]>
---
    drivers/spi/cadence_qspi_apb.c |    7 ++-----
    1 file changed, 2 insertions(+), 5 deletions(-)
(quoted diff) +0 -0

b/drivers/spi/cadence_qspi_apb.c index 00a115f..855e5c7 100644

drivers/spi/cadence_qspi_apb.c +0 -0
@@ -50,7 +50,6 @@
    #define CQSPI_INST_TYPE_QUAD			(2)
Quoted 6 lines
Quoted 6 lines

drivers/spi/cadence_qspi_apb.c int

Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +0 -1
    #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
    #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
    #define CQSPI_DUMMY_BYTES_MAX			(4)

@@ -697,8 +696,7 @@ int

cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 4 lines
Quoted 4 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 4 lines
drivers/spi/cadence_qspi_apb.c +0 -1
    		addr_bytes = cmdlen - 1;
 
    	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase &

CQSPI_INDIRECTTRIGGER_ADDR_MASK),

Quoted 6 lines
Quoted 6 lines

drivers/spi/cadence_qspi_apb.c int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,

Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +2 -1
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase +
+CQSPI_REG_INDIRECTTRIGGER);
 
    	/* Configure SRAM partition for read. */
    	writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase + @@ -

798,8

Quoted 1 line
Quoted 1 line
Quoted 1 line

+796,7 @@ int cadence_qspi_apb_indirect_write_setup(struct

cadence_spi_platdata *plat,

Quoted 4 lines
Quoted 4 lines

drivers/spi/cadence_qspi_apb.c

Quoted 4 lines
    		return -EINVAL;
    	}
    	/* Setup the indirect trigger address */
drivers/spi/cadence_qspi_apb.c +0 -1
-	writel(((u32)plat->ahbbase &

CQSPI_INDIRECTTRIGGER_ADDR_MASK),

Quoted 6 lines
Quoted 5 lines

drivers/spi/cadence_qspi_apb.c

Quoted 3 lines
drivers/spi/cadence_qspi_apb.c +2 -1
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
+	writel((u32)plat->ahbbase, plat->regbase +
+CQSPI_REG_INDIRECTTRIGGER);

Might I ask on which SoC you are using this driver? Which problem are you experiencing exactly?

Quoted 8 lines

I am using ST's soc stv0991. The spi flash memory is mapped at address 0x4000_0000. By Masking with 0xF_FFFF, the value to be loaded in the indirect trigger register becomes zero & then spi read/write does not work. By removing or changing the mask to 0xFFFF_FFFF, the loaded value in indirect trigger register is 0x4000_0000 & spi read/write works fine.

It should be same of socfpga also after masking with 0xF_FFFF, wondering how it works there.

Quoted 8 lines

Yes, I'm wondering a bit myself. Thats why I added Graham to Cc as he is the author of the original driver. And knows this far better than I do.

Quoted 3 lines

No masking is done in the driver while polling fifo for read/write, plat->ahbbase was used directly. Any idea why this masking was done for ahb trigger base register?

Graham? Do you have any insight here please?

Any idea ?

Rgds,
Vikas

Quoted 3 lines

Thanks,
Stefan