Re: [U-Boot] [PATCH] spi: cadence_qspi_apb: Add trigger-base DT bindings from Linux
On 2/28/2017 8:38 PM, Rush, Jason A. wrote: [...] > > This also works. > > Marek - how do you feel about a patch series with the following: > > 1. revert commit 57897c13de03ac0136d64641a3eab526c6810387 > spi: cadence_qspi_apb: Use 32 bit indirect write transaction when > possible > 2. revert commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f > spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible > 3. Apply my slightly modified version of > https://patchwork.ozlabs.org/patch/693069/ > (should I keep Vignesh as the signed-off?) Depends on how much you have changed the code. If the change is significant then change the authorship to you and drop my signed-off. Else, keep the authorship and signed-off. If you are adding something new to the patch like adding code to make sure that only 32bit data reads are issued, then I suggest you to submit that change as separate patch. > 4. Clean-up loading the reg variables to use fdtdec_get_addr_xxx(...) and mark > them all as __iomem > 5. Load the indirect-trigger property from the DT as a u32. I think this > still > needs to be a u32 because it's used in a writel(...) 32-bit write call in > cadence_qspi_apb.c > 6. Add indirect-trigger to dts files that use cadence driver > > I think that captures all the changes needed. > > Also, for the naming of the DT property, Linux has uses a 'cdns,' prefix > (e.g. cdns,trigger-address) for a number of their properties (cdns,tshsl-ns, > cdns,tsd2d-ns, etc.), but u-boot does not currently use the 'cdns,' prefix for > the same properties. Do you want the 'cdns,' prefix on trigger-address? > If so, do you want me to rename all the other properties to align them with > the Linux DT as an additional patch? > > -- > Regards, > Jason > -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: cadence_qspi_apb: Add trigger-base DT bindings from Linux
On 2/25/2017 1:25 AM, Rush, Jason A. wrote: > R, Vignesh wrote: >> On 2/24/2017 12:55 AM, Marek Vasut wrote: >>> On 02/23/2017 08:22 PM, Rush, Jason A. wrote: >>>> Marek Vasut wrote: >>>>> On 02/22/2017 06:37 PM, Rush, Jason A. wrote: >>>>>> Marek Vasut wrote: >>>>>>> On 02/21/2017 05:50 PM, Rush, Jason A. wrote: >> [...] >>>> >> >> You could try reverting my commits: >> commit 57897c13de03ac0136d64641a3eab526c6810387 spi: cadence_qspi_apb: >> Use 32 bit indirect write transaction when possible >> commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f spi: cadence_qspi_apb: >> Use 32 bit indirect read transaction when possible >> > > When I reverted these two commits and added my patch for the indirect > trigger_address, it works correctly. > Oops, these patches are required as Cadence QSPI controller(I am not sure if all versions of IP are newer versions only) has a limitation that the external master is only permitted to issue 32-bit data interface reads until the last word of an indirect transfer. > Also, when I disabled the dcache (dcache off) as Marek suggested, it works > correctly when running from the master branch (again with my indirect > trigger_address patch). > Just that I understand correctly, with latest master(with no patches reverted) + your patch for indirect trigger_address + dcache off, you don't see any problem? >> >> But there were other patches by others in v2017.01-rc1, like >> spi: cadence_qspi: Fix CS timings which may have impact. >> > > I left all other commits in except the two Vignesh suggested to revert, so > it seems to be related to those two commits and caching. As another data > point, I can load and boot linux with caching on from another source (MMC). > So I don't think it's a problem with memory/caching in general. > > Any suggestions on how to proceed from here? > My patches use common bounce_buffer implementation which does dcache flush/invalidate and if dcache has issues then I guess those operations may be causing data corruption. Could you do a bit more research for me? 1. As a hack, could you just disable dcache operations in bounce_buffer implementation? Here is the diff for the same: diff --git a/common/bouncebuf.c b/common/bouncebuf.c index 054d9e0302cc..2878b9eed1ae 100644 --- a/common/bouncebuf.c +++ b/common/bouncebuf.c @@ -55,21 +55,21 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data, * Flush data to RAM so DMA reads can pick it up, * and any CPU writebacks don't race with DMA writes */ - flush_dcache_range((unsigned long)state->bounce_buffer, - (unsigned long)(state->bounce_buffer) + - state->len_aligned); +// flush_dcache_range((unsigned long)state->bounce_buffer, +// (unsigned long)(state->bounce_buffer) + +// state->len_aligned); return 0; } int bounce_buffer_stop(struct bounce_buffer *state) { - if (state->flags & GEN_BB_WRITE) { - /* Invalidate cache so that CPU can see any newly DMA'd data */ - invalidate_dcache_range((unsigned long)state->bounce_buffer, - (unsigned long)(state->bounce_buffer) + - state->len_aligned); - } +// if (state->flags & GEN_BB_WRITE) { +// /* Invalidate cache so that CPU can see any newly DMA'd data */ +// invalidate_dcache_range((unsigned long)state->bounce_buffer, +// (unsigned long)(state->bounce_buffer) + +// state->len_aligned); +// } if (state->bounce_buffer == state->user_buffer) return 0; > > > 2. If that works, I guess there is some issue wrt CQSPI and dcache on your platform, I suggest you to revert my above two patches and try non bounce buffer version of my changes here: https://patchwork.ozlabs.org/patch/693069/. This patch takes care of indirect write. I don't have similar patch for indirect read but that wasn't required. -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: cadence_qspi_apb: Add trigger-base DT bindings from Linux
On 2/24/2017 12:55 AM, Marek Vasut wrote: > On 02/23/2017 08:22 PM, Rush, Jason A. wrote: >> Marek Vasut wrote: >>> On 02/22/2017 06:37 PM, Rush, Jason A. wrote: Marek Vasut wrote: > On 02/21/2017 05:50 PM, Rush, Jason A. wrote: [...] >> >> While I was debugging some of my changes, I noticed that the data being read >> from the >> QSPI flash device appears to be random. The CPU no longer resets while >> performing a >> read when the indirect trigger address is setup correctly for the Altrera >> SoC, but there >> appears to be a larger problem with reading data in general. >> How random is it? Is the problem seen only when unaligned read/write are done or when length of transfer is not a multiple of word(4 byte)? If the data is really random in all cases, then I suspect timing issues. Please see if delay values are populated correctly in DT. >> When I apply my patch to the v2016.11 release, reads appear correct. >> However, when I >> apply my patch to the v2017.01 release, the data read from the QSPI device >> appear to be >> random/corrupt. I noticed the cadence_spi_apb.c file changed significantly >> between >> v2016.11 and v2017.01, possibly a change in this file is causing the problem >> on the Altera >> SoC. >> >> I'm not really that familiar with the cadence device, so this issue is >> getting a little beyond a >> simple patch to setup the indirect trigger address correctly for the Altrera >> SoC. Is there >> anyone more familiar with the cadence device on the Altera SoC that could >> take a look >> into this? > > Vignesh did those changes, so I think he can assist you. In the > meantime, you can try git bisect . Another thing you can try is > disabling the dcache and see if that fixes things (dcache off), > I recall seeing some caching issues with CQSPI. > You could try reverting my commits: commit 57897c13de03ac0136d64641a3eab526c6810387 spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible But there were other patches by others in v2017.01-rc1, like spi: cadence_qspi: Fix CS timings which may have impact. -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RESEND v2 1/2] spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible
On 12/21/2016 10:42 AM, Vignesh R wrote: > According to Section 11.15.4.9.2 Indirect Write Controller of K2G SoC > TRM SPRUHY8D[1], the external master is only permitted to issue 32-bit > data interface writes until the last word of an indirect transfer > otherwise indirect writes is known to fails sometimes. So, make sure > that QSPI indirect writes are 32 bit sized except for the last write. If > the txbuf is unaligned then use bounce buffer to avoid data aborts. > > So, now that the driver uses bounce_buffer, enable CONFIG_BOUNCE_BUFFER > for all boards that use Cadence QSPI driver. > > [1]www.ti.com/lit/ug/spruhy8d/spruhy8d.pdf > > Signed-off-by: Vignesh R > Reviewed-by: Marek Vasut Gentle ping on the series... > --- > > Resend v2: Rebased on latest rc. > Link to v2:https://patchwork.ozlabs.org/patch/698614/ > > drivers/spi/cadence_qspi_apb.c | 26 -- > include/configs/k2g_evm.h| 1 + > include/configs/socfpga_common.h | 1 + > include/configs/stv0991.h| 1 + > 4 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > index df6a91fc9f7b..5d5b6f0d350b 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include "cadence_qspi.h" > > #define CQSPI_REG_POLL_US1 /* 1us */ > @@ -724,6 +725,17 @@ int cadence_qspi_apb_indirect_write_execute(struct > cadence_spi_platdata *plat, > unsigned int remaining = n_tx; > unsigned int write_bytes; > int ret; > + struct bounce_buffer bb; > + u8 *bb_txbuf; > + > + /* > + * Handle non-4-byte aligned accesses via bounce buffer to > + * avoid data abort. > + */ > + ret = bounce_buffer_start(&bb, (void *)txbuf, n_tx, GEN_BB_READ); > + if (ret) > + return ret; > + bb_txbuf = bb.bounce_buffer; > > /* Configure the indirect read transfer bytes */ > writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES); > @@ -734,11 +746,11 @@ int cadence_qspi_apb_indirect_write_execute(struct > cadence_spi_platdata *plat, > > while (remaining > 0) { > write_bytes = remaining > page_size ? page_size : remaining; > - /* Handle non-4-byte aligned access to avoid data abort. */ > - if (((uintptr_t)txbuf % 4) || (write_bytes % 4)) > - writesb(plat->ahbbase, txbuf, write_bytes); > - else > - writesl(plat->ahbbase, txbuf, write_bytes >> 2); > + writesl(plat->ahbbase, bb_txbuf, write_bytes >> 2); > + if (write_bytes % 4) > + writesb(plat->ahbbase, > + bb_txbuf + rounddown(write_bytes, 4), > + write_bytes % 4); > > ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL, > CQSPI_REG_SDRAMLEVEL_WR_MASK << > @@ -748,7 +760,7 @@ int cadence_qspi_apb_indirect_write_execute(struct > cadence_spi_platdata *plat, > goto failwr; > } > > - txbuf += write_bytes; > + bb_txbuf += write_bytes; > remaining -= write_bytes; > } > > @@ -759,6 +771,7 @@ int cadence_qspi_apb_indirect_write_execute(struct > cadence_spi_platdata *plat, > printf("Indirect write completion error (%i)\n", ret); > goto failwr; > } > + bounce_buffer_stop(&bb); > > /* Clear indirect completion status */ > writel(CQSPI_REG_INDIRECTWR_DONE, > @@ -769,6 +782,7 @@ failwr: > /* Cancel the indirect write */ > writel(CQSPI_REG_INDIRECTWR_CANCEL, > plat->regbase + CQSPI_REG_INDIRECTWR); > + bounce_buffer_stop(&bb); > return ret; > } > > diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h > index 2da0d8dd7f00..4f9c42abac7e 100644 > --- a/include/configs/k2g_evm.h > +++ b/include/configs/k2g_evm.h > @@ -78,6 +78,7 @@ > #define CONFIG_CADENCE_QSPI > #define CONFIG_CQSPI_REF_CLK 38400 > #define CONFIG_CQSPI_DECODER 0x0 > +#define CONFIG_BOUNCE_BUFFER > #endif > > #endif /* __CONFIG_K2G_EVM_H */ > diff --git a/include/configs/socfpga_common.h > b/include/configs/socfpga_common.h > index 58a655084491..e50c2fac8d99 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -208,6 +208,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); > #define CONFIG_CQSPI_REF_CLK cm_get_qspi_controller_clk_hz() > #endif > #define CONFIG_CQSPI_DECODER 0 > +#define CONFIG_BOUNCE_BUFFER > > /* > * Designware SPI support > diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h > index bfd1bd719285..09a3064bd6d6 100644 > --- a/include/configs/stv0991.h > +++ b/include/configs/stv0991.h > @@ -74,6 +74,7 @@ > #ifdef C
Re: [U-Boot] [PATCH RESEND v2 2/2] spi: ti_qspi: Fix baudrate divider calculation
[...] On 11/4/2016 4:31 PM, Jagan Teki wrote: >>> >> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>> >> index 52520dff6325..b5de70bf40e3 100644 >>> >> --- a/drivers/spi/ti_qspi.c >>> >> +++ b/drivers/spi/ti_qspi.c >>> >> @@ -16,6 +16,7 @@ >>> >> #include >>> >> #include >>> >> #include >>> >> +#include >>> >> >>> >> DECLARE_GLOBAL_DATA_PTR; >>> >> >>> >> @@ -118,21 +119,19 @@ static void ti_spi_set_speed(struct ti_qspi_priv >>> >> *priv, uint hz) >>> >> if (!hz) >>> >> clk_div = 0; >>> >> else >>> >> - clk_div = (priv->fclk / hz) - 1; >>> >> - >>> >> - debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, >>> >> clk_div); >>> >> + clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1; >>> >> >>> >> /* disable SCLK */ >>> >> writel(readl(&priv->base->clk_ctrl) & ~QSPI_CLK_EN, >>> >>&priv->base->clk_ctrl); >> > >> > Move this before enable SCLK. Ok... > Do send the updated v3 or discusses further, I need to send the release PR? Sorry for the delay.. Posted v3 with above change. -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RESEND v2 1/2] ARM: dts: dra7xx: Update spi-max-frequency for qspi slave node
On 10/31/2016 5:24 PM, Tom Rini wrote: > On Mon, Oct 31, 2016 at 09:40:34AM +0530, Vignesh R wrote: > >> Update the spi-max-frequency property of m25p80 flash slave to match >> that of TI QSPI controller node, so that QSPI operations happen at >> maximum supported frequency of 76.8MHz. >> >> Signed-off-by: Vignesh R >> Reviewed-by: Jagan Teki > > And this is also done in the kernel right? Thanks! > Yes, the kernel patch is merged. -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation
On 10/14/2016 12:27 PM, Jagan Teki wrote: > On Fri, Oct 14, 2016 at 10:54 AM, Vignesh R wrote: ... DECLARE_GLOBAL_DATA_PTR; @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz) if (!hz) clk_div = 0; else - clk_div = (priv->fclk / hz) - 1; + clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1; >>> >>> Better to have a checks for min and max divider values or mask. >> >> That code already exists in this function. > > True but it's unnecessary to print the wrong baud prior to checking. > Do the check, then print/debug and finally write reg. > Posted a v2 in reply to the patch. Thanks for the review! Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/4] net: cpsw: Add support to drive gpios for ethernet to be functional
On 7/26/2016 12:26 PM, Wolfgang Denk wrote: > Dear "R, Vignesh", > > In message you wrote: >> >>>> @@ -1203,6 +1206,16 @@ static int cpsw_eth_ofdata_to_platdata(struct >>>> udevice *dev) >>>>return -ENOENT; >>>>} >>>> >>>> + >>>> + num_mode_gpios = gpio_get_list_count(dev, "mode-gpios"); >>> >>> Extra blank line added. >> >> I added blank line for readability. So, that code block handling >> toggling of "mode-gpios" is separated from the rest. I can remove that >> if that seems unnecessary. > > There was already a blank line which provides enough separation. No > need to have multiple consecutive blank lines. Sorry, will clean that up in v2. Thanks! Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/4] net: cpsw: Add support to drive gpios for ethernet to be functional
On 7/25/2016 7:08 PM, Tom Rini wrote: > On Mon, Jul 25, 2016 at 06:40:22PM +0530, Vignesh R wrote: > >> On DRA72 EVM, cpsw slaves may be muxed with other modules. This >> selection is controlled by a pcf gpio line. Add support for cpsw driver >> to acquire mode-gpios and select the appropriate slave using gpio APIs. >> >> Signed-off-by: Vignesh R > > Reviewed-by: Tom Rini > > Minor nit below: > >> @@ -1203,6 +1206,16 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice >> *dev) >> return -ENOENT; >> } >> >> + >> +num_mode_gpios = gpio_get_list_count(dev, "mode-gpios"); > > Extra blank line added. I added blank line for readability. So, that code block handling toggling of "mode-gpios" is separated from the rest. I can remove that if that seems unnecessary. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] spi: ti_qspi: Fix failure on multiple READ_ID cmd
On 7/11/2016 12:05 PM, Jagan Teki wrote: > On 11 July 2016 at 11:00, Vignesh R wrote: >> Populating QSPI_RD_SNGL bit(0x1) in priv->cmd means that value >> QSPI_INVAL (0x4) is not written to CMD field of QSPI_SPI_CMD_REG in >> ti_qspi_cs_deactivate(). Therefore CS is never deactivated between >> successive READ ID which results in sf probe to fail. >> Fix this by not populating priv->cmd with QSPI_RD_SNGL and OR it wih >> priv->cmd as required (similar to the convention followed in the >> driver). >> >> Signed-off-by: Vignesh R >> --- >> drivers/spi/ti_qspi.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >> index 9a372ad31dae..376fe378ed63 100644 >> --- a/drivers/spi/ti_qspi.c >> +++ b/drivers/spi/ti_qspi.c >> @@ -247,13 +247,12 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, >> unsigned int bitlen, >> debug("tx done, status %08x\n", status); >> } >> if (rxp) { >> - priv->cmd |= QSPI_RD_SNGL; >> debug("rx cmd %08x dc %08x\n", >> priv->cmd, priv->dc); > > | QSPI_RD_SNGL on debug statement as well. Thanks, will fix this in v2. > >> #ifdef CONFIG_DRA7XX >> udelay(500); >> #endif > > Can't we fix this delay, still need? The patch that added this delay ( b545a98f5dc563 spi: ti_qspi: Add delay for successful bulk erase) says its added to meet bulk erase timing constraints (AFAIK, I believe bulk erase is same as chip erase which is not supported by sf erase but may be supported in future). I will experiment a bit and convince myself whether this delay is really applicable or not. Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] dm: core: implement dev_map_phsymem()
On 5/6/2016 9:00 PM, Jagan Teki wrote: > On 6 May 2016 at 09:28, Vignesh R wrote: >> This API helps to map physical register addresss pace of device to >> virtual address space easily. Its just a wrapper around map_physmem() >> with MAP_NOCACHE flag. >> >> Signed-off-by: Vignesh R >> Suggested-by: Simon Glass >> Reviewed-by: Jagan Teki >> >> --- >> >> v4: Reorder include files to avoid build warning on dra7xx. >> >> drivers/core/device.c | 6 ++ >> include/dm/device.h | 9 + >> 2 files changed, 15 insertions(+) >> >> diff --git a/drivers/core/device.c b/drivers/core/device.c >> index 1322991d6c7b..6b19b4b8c7a0 100644 >> --- a/drivers/core/device.c >> +++ b/drivers/core/device.c >> @@ -10,6 +10,7 @@ >> */ >> >> #include >> +#include > > I think this look same as v3 [1] please check? Yeah, v3 has the include files in correct. Please ignore this patch. Sorry for the spam. Regards Vignesh. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] QSPI XIP boot on am437x
Hi Albert, Thanks for the response! On 11/10/2015 5:44 PM, Albert ARIBAUD wrote: > Hello Vignesh, > > On Tue, 10 Nov 2015 14:29:54 +0530, Vignesh R wrote: >> Hi, >> >> With commit 7ae8350f67eea("ti: armv7: Move SPL SDRAM init to the right >> place, drop unused CONFIG_SPL_STACK") QSPI XIP boot appears to be broken >> on AM437x SK EVM. >> >> Following UART initialization code (as indicated by TODO) causes the XIP >> boot failure. >> >> >> In arch/arm/cpu/armv7/am33xx/board.c: >> @@ -275,9 +275,9 @@ void s_init(void) >> #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT) >> /* TODO: This does not work, gd is not available yet */ >>gd->baudrate = CONFIG_BAUDRATE; >>serial_init(); >>gd->have_console = 1; >> #endif >> >> >> I was able to boot successfully from QSPI by commenting out the above code. >> But, could you suggest me what needs to be done as part of TODO in order >> to get QSPI XIP boot working? > > Can't answer specifically on am437x, but basically, the problem you > have here may be that the code is running in a C runtime environment in > which only the global data is writable. This global data is a struct > global_data (see include/asm-generic/global_data.h) which is supposed > to be pointed to by the variable GD. > > Can you detail the failure you are encountering? > > Typically, GD is set up from within function board_init_f_mem(), before > calling board_init_f(), or from arch/arm/lib/crt0.S. > > So all depends on whether s_init() is executed before or after > board_init_f_mem(). > > If s_init() runs before board_init_f_mem(), then you must move it to > run after board_init_f_mem(). :) > > If s_init() runs after board_init_f_mem() and you still have the issue, > then your problem would be that gd is badly initialized. Is your board > built for Thumb with a recent compiler, by any chance? I any case, can > you test the value of gd when reaching the gd->baudrate line above? Yes, s_init() is being called before call to _main(in arch/arm/lib/crt0.S that sets up GD) but all these calls are from arm generic files and nothing specific to am437x: reset (arch/arm/cpu/armv7/start.S) -> cpu_init_crit(arch/arm/cpu/armv7/start.S) -> lowlevel_init(arch/arm/cpu/armv7/lowlevel_init.S) -> s_init(arch/arm/cpu/armv7/board/am33xx.c) The failure appears to be in serial_init(), it tries to access gd->flags which is not allocated yet and reads wrong value. I was wondering whether entire UART initialization code in s_init() in arch/arm/cpu/armv7/board/am33xx.c can be moved to the end of board_init_f() where GD is accessible. Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [U-Boot RESEND v2 09/10] spi: ti_qspi: Use DMA to read from qspi flash
On 8/17/2015 1:48 PM, Jagan Teki wrote: > On 17 August 2015 at 13:29, Vignesh R wrote: >> ti_qspi uses memory map mode for faster read. Enabling DMA will increase >> read speed by 3x @48MHz on DRA74 EVM. >> >> Signed-off-by: Vignesh R >> Reviewed-by: Jagan Teki >> --- >> drivers/spi/ti_qspi.c | 23 +++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >> index 3356c0f072e5..753d68980bd6 100644 >> --- a/drivers/spi/ti_qspi.c >> +++ b/drivers/spi/ti_qspi.c >> @@ -13,6 +13,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> >> /* ti qpsi register bit masks */ >> #define QSPI_TIMEOUT200 >> @@ -347,3 +349,24 @@ int spi_xfer(struct spi_slave *slave, unsigned int >> bitlen, const void *dout, >> >> return 0; >> } > Please add below comment here, I have asked the same on previous version patch > this will track us to the work future. > > /* TODO: control from sf layer to here through dm-spi */ Oops.. Sorry, I overlooked it.. Will add the comment and send it soon. Thanks! >> +#ifdef CONFIG_TI_EDMA3 >> +void spi_flash_copy_mmap(void *data, void *offset, size_t len) >> +{ >> + unsigned intaddr = (unsigned int) (data); >> + unsigned intedma_slot_num = 1; >> + >> + /* Invalidate the area, so no writeback into the RAM races with DMA >> */ >> + invalidate_dcache_range(addr, addr + roundup(len, >> ARCH_DMA_MINALIGN)); >> + >> + /* enable edma3 clocks */ >> + enable_edma3_clocks(); >> + >> + /* Call edma3 api to do actual DMA transfer */ >> + edma3_transfer(EDMA3_BASE, edma_slot_num, data, offset, len); >> + >> + /* disable edma3 clocks */ >> + disable_edma3_clocks(); >> + >> + *((unsigned int *)offset) += len; >> +} >> +#endif >> -- >> 2.5.0 > > thanks! > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/11] spi: ti_qspi: Use DMA to read from qspi flash
On 7/15/2015 12:32 AM, Tom Rini wrote: > On Thu, Jul 09, 2015 at 12:10:03PM +0530, Vignesh R wrote: >> >> >> On 07/03/2015 05:12 PM, Tom Rini wrote: >>> On Fri, Jul 03, 2015 at 04:46:10PM +0530, Vignesh R wrote: >>> ti_qspi uses memory map mode for faster read. Enabling DMA will increase read speed by 3x @48MHz on DRA74 EVM. Signed-off-by: Vignesh R >>> >>> This ignores the feedback from >>> http://lists.denx.de/pipermail/u-boot/2014-July/183715.html where we >>> need to model the DMA changes on how it's done for mxs_spi.c >>> >> Is the following patch an acceptable solution? >> > > Jagan, are you OK with the SPI side of this? Thanks! Gentle ping... Any comments? I will send a v2 for this series if the below patch is acceptable. > >> 8<--- >> >> Move DMA related initialization code to helper function in ti-edma3 >> driver. Use this function for scheduling DMA transfer from ti_qspi driver. >> >> diff --git a/arch/arm/include/asm/ti-common/ti-edma3.h >> b/arch/arm/include/asm/ti-common/ti-edma3.h >> index 5adc1dac0e65..6a7a321c1bdf 100644 >> --- a/arch/arm/include/asm/ti-common/ti-edma3.h >> +++ b/arch/arm/include/asm/ti-common/ti-edma3.h >> @@ -117,5 +117,7 @@ void edma3_set_src_addr(u32 base, int slot, u32 src); >> void edma3_set_transfer_params(u32 base, int slot, int acnt, >> int bcnt, int ccnt, u16 bcnt_rld, >> enum edma3_sync_dimension sync_mode); >> +void edma3_transfer(unsigned long edma3_base_addr, unsigned int >> +edma_slot_num, void *dst, void *src, size_t len); >> >> #endif >> diff --git a/drivers/dma/ti-edma3.c b/drivers/dma/ti-edma3.c >> index 8184ded9fa81..d6a427f2e21d 100644 >> --- a/drivers/dma/ti-edma3.c >> +++ b/drivers/dma/ti-edma3.c >> @@ -382,3 +382,81 @@ void qedma3_stop(u32 base, struct >> edma3_channel_config *cfg) >> /* Clear the channel map */ >> __raw_writel(0, base + EDMA3_QCHMAP(cfg->chnum)); >> } >> + >> +void edma3_transfer(unsigned long edma3_base_addr, unsigned int >> +edma_slot_num, void *dst, void *src, size_t len) >> +{ >> +struct edma3_slot_configslot; >> +struct edma3_channel_config edma_channel; >> +int b_cnt_value = 1; >> +int rem_bytes = 0; >> +int a_cnt_value = len; >> +unsigned intaddr = (unsigned int) (dst); >> +unsigned intmax_acnt = 0x7FFFU; >> + >> +if (len > max_acnt) { >> +b_cnt_value = (len / max_acnt); >> +rem_bytes = (len % max_acnt); >> +a_cnt_value = max_acnt; >> +} >> + >> +slot.opt= 0; >> +slot.src= ((unsigned int) src); >> +slot.acnt = a_cnt_value; >> +slot.bcnt = b_cnt_value; >> +slot.ccnt = 1; >> +slot.src_bidx = a_cnt_value; >> +slot.dst_bidx = a_cnt_value; >> +slot.src_cidx = 0; >> +slot.dst_cidx = 0; >> +slot.link = EDMA3_PARSET_NULL_LINK; >> +slot.bcntrld= 0; >> +slot.opt= EDMA3_SLOPT_TRANS_COMP_INT_ENB | >> + EDMA3_SLOPT_COMP_CODE(0) | >> + EDMA3_SLOPT_STATIC | EDMA3_SLOPT_AB_SYNC; >> + >> +edma3_slot_configure(edma3_base_addr, edma_slot_num, &slot); >> +edma_channel.slot = edma_slot_num; >> +edma_channel.chnum = 0; >> +edma_channel.complete_code = 0; >> + /* set event trigger to dst update */ >> +edma_channel.trigger_slot_word = EDMA3_TWORD(dst); >> + >> +qedma3_start(edma3_base_addr, &edma_channel); >> +edma3_set_dest_addr(edma3_base_addr, edma_channel.slot, addr); >> + >> +while (edma3_check_for_transfer(edma3_base_addr, &edma_channel)) >> +; >> +qedma3_stop(edma3_base_addr, &edma_channel); >> + >> +if (rem_bytes != 0) { >> +slot.opt= 0; >> +slot.src= >> +(b_cnt_value * max_acnt) + ((unsigned int) src); >> +slot.acnt = rem_bytes; >> +slot.bcnt = 1; >> +slot.ccnt = 1; >> +slot.src_bidx = rem_bytes; >> +slot.dst_bidx = rem_bytes; >> +slot.src_cidx = 0; >> +slot.dst_cidx = 0; >> +slot.link = EDMA3_PARSET_NULL_LINK; >> +slot.bcntrld= 0; >> +slot.opt= EDMA3_SLOPT_TRANS_COMP_INT_ENB | >> + EDMA3_SLOPT_COMP_CODE(0) | >> + EDMA3_SLOPT_STATIC | EDMA3_SLOPT_AB_SYNC; >> +edma3_slot_configure(edma3_base_addr, edma_slot_num, &slot); >> +edma_channel.slot = edma_slot_num; >> +edma_channel.chnum = 0; >> +edma_channel.complete_code = 0; >> +/* set event trigger to dst update */ >> +edma_channel.trigger_slot_word = E
Re: [U-Boot] [PATCH 09/11] dma: ti-edma3: Add BIT(x) macro definition
On 7/3/2015 7:27 PM, Andy Pont wrote: > Vignesh wrote... > > [snip] > >> +#define BIT(x) (1 << (x)) >> + > > Is this not something that would be better in a global header file somewhere > rather than it starting a trend of a per-driver, per-arch, etc. definitions? I agree there are few per arch defintions. How about adding it to include/linux/bitops.h as in linux kernel? Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/11] spi: ti_qspi: Use DMA to read from qspi flash
On 7/3/2015 5:12 PM, Tom Rini wrote: > On Fri, Jul 03, 2015 at 04:46:10PM +0530, Vignesh R wrote: > >> ti_qspi uses memory map mode for faster read. Enabling DMA will increase >> read speed by 3x @48MHz on DRA74 EVM. >> >> Signed-off-by: Vignesh R > > This ignores the feedback from > http://lists.denx.de/pipermail/u-boot/2014-July/183715.html where we > need to model the DMA changes on how it's done for mxs_spi.c > Sorry.. I didn't look into that before. mxs_spi uses peripheral DMA to read/write flash. But ti_qspi can use DMA to read from flash in mmap mode only. In current u-boot, defining CONFIG_TI_SPI_MMAP will make memory map address available (spi_flash->memory_map) to sf layer and spi_flash_cmd_read_ops() (in sf_ops.c) directly calls memcpy() to read data from flash into buffer. There is no spi_xfer() call to the ti_qspi driver at all. In order to implement mxs_spi like approach for ti_qspi.c, I can delete mmap handling in sf_ops.c( I don't think any other spi driver uses this part of code), so that spi_xfer() is always called. And then, in spi_xfer() implementation of ti_qspi, I can do DMA transfer similar to mxs_spi.c. Is this approach ok? And are you ok with patch 1 and 2 of this series? Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot