[U-Boot] [PATCH] spi: cadence_qspi: Fix the indirect ahb trigger address setting

[Vikas MANOCHA]

Hi Graham,

> -----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,
> 
> On 29.05.2015 18:53, vikasm wrote:
> > On 05/28/2015 11:20 PM, Stefan Roese wrote:
> >> Hi Vikas,
> >>
> >> (added Dinh, Graham and Marek to Cc)
> >>
> >> On 29.05.2015 03:22, Vikas Manocha wrote:
> >>> 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 <vikas.manocha at st.com>
> >>> ---
> >>>    drivers/spi/cadence_qspi_apb.c |    7 ++-----
> >>>    1 file changed, 2 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/spi/cadence_qspi_apb.c
> >>> b/drivers/spi/cadence_qspi_apb.c index 00a115f..855e5c7 100644
> >>> --- a/drivers/spi/cadence_qspi_apb.c
> >>> +++ b/drivers/spi/cadence_qspi_apb.c
> >>> @@ -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);
> >> 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.
> 
> 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.
> 
> > 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

> 
> Thanks,
> Stefan