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