From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Mon, 01 Jun 2015 13:34:09 +0200
Subject: [U-Boot] [PATCH] spi: cadence_qspi: Fix the indirect ahb
 trigger address setting
In-Reply-To: <5568999B.1090307@st.com>
References: <1432862544-20398-1-git-send-email-vikas.manocha@st.com>
 <55680532.70209@denx.de> <5568999B.1090307@st.com>
Message-ID: <556C4331.6030908@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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?

Thanks,
Stefan