diff mbox series

[U-Boot] arm socfpga: Revert "spi: cadence_qspi_apb: Support 32 bit AHB address"

Message ID 5a241c93.4d2bc80a.49dc7.267d@mx.google.com
State Deferred
Delegated to: Marek Vasut
Headers show
Series [U-Boot] arm socfpga: Revert "spi: cadence_qspi_apb: Support 32 bit AHB address" | expand

Commit Message

Frank Mori Hess Dec. 3, 2017, 3:36 p.m. UTC
This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Signed-off-by: Frank Mori Hess <fmh6jj@gmail.com>
---
 drivers/spi/cadence_qspi_apb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Fabio Estevam Dec. 3, 2017, 3:49 p.m. UTC | #1
On Sun, Dec 3, 2017 at 1:36 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
> This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.

Please explain the reasoning for the revert.
Frank Mori Hess Dec. 3, 2017, 3:50 p.m. UTC | #2
Oops, ignore that patch I didn't merge it correctly

On Sun, Dec 3, 2017 at 10:36 AM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
> This reverts commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1.
>
> Signed-off-by: Frank Mori Hess <fmh6jj@gmail.com>
> ---
>  drivers/spi/cadence_qspi_apb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index e02f2217f4..b300f36607 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -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
>
>
Frank Mori Hess Dec. 3, 2017, 3:51 p.m. UTC | #3
On Sun, Dec 3, 2017 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sun, Dec 3, 2017 at 1:36 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
>> 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 <fmh6jj@gmail.com>
Date: Sat, Dec 2, 2017 at 7:50 PM
Subject: commit dac3bf20fb2c9b03476be0d73db620f62ab3cee1 breaks cadence driver
To: u-boot@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
Fabio Estevam Dec. 3, 2017, 3:54 p.m. UTC | #4
On Sun, Dec 3, 2017 at 1:51 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
> On Sun, Dec 3, 2017 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Sun, Dec 3, 2017 at 1:36 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
>>> 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.
Frank Mori Hess Dec. 3, 2017, 4:42 p.m. UTC | #5
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:

>>  /* 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.


On Sun, Dec 3, 2017 at 10:54 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sun, Dec 3, 2017 at 1:51 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
>> On Sun, Dec 3, 2017 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
>>> On Sun, Dec 3, 2017 at 1:36 PM, Frank Mori Hess <fmh6jj@gmail.com> wrote:
>>>> 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.
diff mbox series

Patch

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e02f2217f4..b300f36607 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -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 */