Skip to contentSkip to conversation

arm socfpga: Revert "spi: cadence_qspi_apb: Support 32 bit AHB address"

U-Boot Frank Mori Hess 2 participants 6 messages Dec 3, 2017 SPI

lore
Share & tools
lore

Frank Mori Hess

author
#0 lore raw

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Signed-off-by Frank Mori Hess <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/spi/cadence_qspi_apb.c +3 -2
index e02f2217f4..b300f36607 100644
@@ -47,6 +47,7 @@
 #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

@@ -560,7 +561,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,
+	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
 	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Configure the opcode */

@@ -710,7 +711,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 		return -EINVAL;
 	}
 	/* Setup the indirect trigger address */
-	writel((u32)plat->ahbbase,
+	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
 	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Configure the opcode */
Signature · 1 line
2.11.0
#1 lore raw
Quoted 1 line · Frank Mori Hess

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Please explain the reasoning for the revert.

author
#2 lore raw
Quoted 4 lines · Fabio Estevam
Quoted 1 line · Frank Mori Hess

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Please explain the reasoning for the revert.

It looks like my original post got stuck in moderation:

---------- Forwarded message ----------
From: Frank Mori Hess <[email protected]>
Date: Sat, Dec 2, 2017 at 7:50 PM
Subject: commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1 breaks cadence driver
To: u-boot at lists.denx.de

Hi,

I've been debugging why u-boot spl crashes in a loop when I boot off a cadence qspi flash. I narrowed it down to the changes from commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1 which removes CQSPI_INDIRECTTRIGGER_ADDR_MASK. Restoring the mask allows the spl to successfully load the main u-boot. My board is an Altera HPS cyclone V socfpga. It has an ahb base address of 0xffa00000 and for some reason, without the CQSPI_INDIRECTTRIGGER_ADDR_MASK the board reboots when cadence_qspi_apb_indirect_read_execute tries to read from the ahb base address. I'm using version 2016.11 of u-boot.

--
Frank

Signature · 5 lines
Frank


-- 
Frank
#3 lore raw
Quoted 7 lines · Frank Mori Hess
Quoted 4 lines · Fabio Estevam
Quoted 1 line · Frank Mori Hess

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Please explain the reasoning for the revert.

It looks like my original post got stuck in moderation:

You should explain inside the commit log why you think it is a good idea to do the revert.

author
#4 lore raw

It looks like the change I'm trying to revert got rejected a couple years ago in another form:

https://lists.denx.de/pipermail/u-boot/2015-August/224556.html

In particular at the end Marek says:

Quoted 10 lines
Quoted 5 lines

/* Indirect mode configurations */ writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
- writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
+ writel((u32)plat->trigger_base,
plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Here you actually changed to logic of the code, which breaks it for SoCFPGA. plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK = 0x0 for SoCFPGA, but now you changed it such that 0xffa00000 is written into the register. Same does apply for all your changes below.

Quoted 11 lines · Fabio Estevam
Quoted 7 lines · Frank Mori Hess
Quoted 4 lines · Fabio Estevam
Quoted 1 line · Frank Mori Hess

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Please explain the reasoning for the revert.

It looks like my original post got stuck in moderation:

You should explain inside the commit log why you think it is a good idea to do the revert.

Signature · 1 line
Frank
author
#5 lore raw
In reply to Frank Mori Hess

Oops, ignore that patch I didn't merge it correctly

Quoted 41 lines · Frank Mori Hess

This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Signed-off-by Frank Mori Hess <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/spi/cadence_qspi_apb.c +3 -3
index e02f2217f4..b300f36607 100644
@@ -47,6 +47,7 @@
 #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

@@ -560,7 +561,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,
+       writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
               plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
        /* Configure the opcode */

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

2.11.0

Signature · 1 line
Frank