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

[vikasm]

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.

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;
>>  }
> 
> [...]
> .
>