[U-Boot] [v2 5/6] spi: cadence_qspi: fix base trigger address & transfer start address

[vikas]

Hi Marek,

On 08/13/2015 09:42 AM, vikasm wrote:
> Hi Marek,
> 
> On 08/12/2015 07:15 PM, Marek Vasut wrote:
>> On Thursday, July 16, 2015 at 04:27:33 AM, Vikas Manocha wrote:
>>> This patch is to separate the base trigger from the read/write transfer
>>> start addresses.
>>
>> This patch breaks the QSPI support on SoCFPGA.
> 
> ok, can you please try to debug the issue. Logically this patch looks good to me unless there
> is something specific to socfpga.

One quick check, can you test if it works on reverting the trigger base in arch/arm/dts/socfpga.dtsi from
<0x00000000 0x0010> to <0xffa00000 0x1000>.

Rgds,
Vikas

> 
> I think latest linux v2 patch from Graham has implemented the same patch.
> 
>>
>>> Base trigger register address (0x1c register) corresponds to the address
>>> which should be put on AHB bus to handle indirect transfer triggered
>>> before.
>>>
>>> To handle indirect transfer we need to issue addresses from (value of 0x1c)
>>> to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM location).
>>> There are no obstacles in issuing const address just equal to 0x1c.
>>> Important thing to note is that indirect trigger address has nothing in
>>> common with your physical or mapped NOR Flash address.
>>>
>>> Transfer read/write start addresses (offset 0x68/0x78)should be programmed
>>> with the absolute flash address to be read/written.
>>>
>>> plat->ahbbase has been renamed to plat->flashbase for clarity.
>>> plat->triggerbase is added in device tree for mapped spi flash address.
>>>
>>> Signed-off-by: Vikas Manocha <vikas.manocha at st.com>
>>> ---
>>>
>>> Changes in v2: Rebased to master
>>>
>>>  arch/arm/dts/socfpga.dtsi      |    3 ++-
>>>  arch/arm/dts/stv0991.dts       |    3 ++-
>>>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
>>>  drivers/spi/cadence_qspi.h     |    5 +++--
>>>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
>>>  5 files changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
>>> index 9b12420..1099a92 100644
>>> --- a/arch/arm/dts/socfpga.dtsi
>>> +++ b/arch/arm/dts/socfpga.dtsi
>>> @@ -633,7 +633,8 @@
>>>  			#address-cells = <1>;
>>>  			#size-cells = <0>;
>>>  			reg = <0xff705000 0x1000>,
>>> -				<0xffa00000 0x1000>;
>>> +				<0xffa00000 0x1000>,
>>> +				<0x00000000 0x0010>;
>>
>> Shouldn't there be a phandle to
>>
>>>  			interrupts = <0 151 4>;
>>>  			clocks = <&qspi_clk>;
>>>  			ext-decoder = <0>;  /* external decoder */
>>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
>>> index fa3fd64..e23d4fd 100644
>>> --- a/arch/arm/dts/stv0991.dts
>>> +++ b/arch/arm/dts/stv0991.dts
>>> @@ -30,7 +30,8 @@
>>>  			#address-cells = <1>;
>>>  			#size-cells = <0>;
>>>  			reg = <0x80203000 0x100>,
>>> -				<0x40000000 0x1000000>;
>>> +				<0x40000000 0x1000000>,
>>> +				<0x40000000 0x0000010>;
>>>  			clocks = <3750000>;
>>>  			sram-size = <256>;
>>>  			status = "okay";
>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>>> index 34a0f46..95c9cea 100644
>>> --- a/drivers/spi/cadence_qspi.c
>>> +++ b/drivers/spi/cadence_qspi.c
>>> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
>>>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
>>>
>>>  	priv->regbase = plat->regbase;
>>> -	priv->ahbbase = plat->ahbbase;
>>> +	priv->flashbase = plat->flashbase;
>>>
>>>  	if (!priv->qspi_is_init) {
>>>  		cadence_qspi_apb_controller_init(plat);
>>> @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct
>>> udevice *bus) const void *blob = gd->fdt_blob;
>>>  	int node = bus->of_offset;
>>>  	int subnode;
>>> -	u32 data[4];
>>> +	u32 data[6];
>>>  	int ret;
>>>
>>>  	/* 2 base addresses are needed, lets get them from the DT */
>>> @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct
>>> udevice *bus) }
>>>
>>>  	plat->regbase = (void *)data[0];
>>> -	plat->ahbbase = (void *)data[2];
>>> +	plat->flashbase = (void *)data[2];
>>> +	plat->trigger_base = (void *)data[4];
>>>
>>>  	/* Use 500KHz as a suitable default */
>>>  	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
>>> @@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct
>>> udevice *bus) plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns",
>>> 20);
>>>  	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
>>>
>>> -	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
>>> -	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
>>> -	      plat->page_size);
>>> -
>>> +	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d \
>>> +		page-size=%d\n", __func__, plat->regbase, plat->flashbase,
>>
>> Please never break formating strings, they are the only exception from the 80 
>> alignment rule. It is not possible to grep for them if they're broken.
> 
> ok, i will fix in next version.
> 
> Rgds,
> Vikas
> 
>>
>>> +		plat->trigger_base, plat->max_hz, plat->page_size);
>>>  	return 0;
>>>  }
>>
>> [...]
>> .
>>