| Message ID | 1440629070-9060-4-git-send-email-vikas.manocha@st.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On Thursday, August 27, 2015 at 12:44:28 AM, Vikas Manocha wrote: > This patch is to separate the base trigger from the read/write transfer > start addresses. > > 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->triggerbase is added in device tree for mapped spi flash address. > > Signed-off-by: Vikas Manocha <vikas.manocha@st.com> > --- > > Changes in v5: None. > Changes in v4: > - fifo-width & trigger address alligned to linux device tree binding. > - renaming of one parameter moved to separate patch. > - trigger address of socfpga reverted back to 0x0. > > Changes in v3: formatted string breaking fixed. > Changes in v2: Rebased to master > > arch/arm/dts/socfpga.dtsi | 1 + > arch/arm/dts/stv0991.dts | 1 + > drivers/spi/cadence_qspi.c | 9 +++++---- > drivers/spi/cadence_qspi.h | 1 + > drivers/spi/cadence_qspi_apb.c | 7 +++---- > 5 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi > index 9b12420..fe6ad80 100644 > --- a/arch/arm/dts/socfpga.dtsi > +++ b/arch/arm/dts/socfpga.dtsi > @@ -639,6 +639,7 @@ > ext-decoder = <0>; /* external decoder */ > num-cs = <4>; > fifo-depth = <128>; > + cdns,trigger-address = <0x0 0x0010>; So you coin this DT prop as a touple here, but in Linux it's has a single element. This will make dealing with the bindings an outright horror. > sram-size = <128>; > bus-num = <2>; > status = "disabled"; > diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts > index fa3fd64..192d9e5 100644 > --- a/arch/arm/dts/stv0991.dts > +++ b/arch/arm/dts/stv0991.dts > @@ -33,6 +33,7 @@ > <0x40000000 0x1000000>; > clocks = <3750000>; > sram-size = <256>; > + cdns,trigger-address = <0x40000000 0x0000010>; > status = "okay"; > > flash0: n25q32@0 { > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 34a0f46..0d1abc8 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -290,6 +290,8 @@ static int cadence_spi_ofdata_to_platdata(struct > udevice *bus) > > plat->regbase = (void *)data[0]; > plat->ahbbase = (void *)data[2]; > + plat->trigger_base = (u32 *)fdtdec_get_addr(blob, node, > + "cdns,trigger-address"); Drop the cast. > /* Use 500KHz as a suitable default */ > plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", > @@ -311,10 +313,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 ahbbase=%p trigger_base=%p max-frequency=%d > page-size=%d\n", + __func__, plat->regbase, plat->ahbbase, > plat->trigger_base, + plat->max_hz, plat->page_size); > return 0; > } > > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > index 98e57aa..2f1bd92 100644 > --- a/drivers/spi/cadence_qspi.h > +++ b/drivers/spi/cadence_qspi.h > @@ -18,6 +18,7 @@ struct cadence_spi_platdata { > unsigned int max_hz; > void *regbase; > void *ahbbase; > + void *trigger_base; > > u32 page_size; > u32 block_size; > diff --git a/drivers/spi/cadence_qspi_apb.c > b/drivers/spi/cadence_qspi_apb.c index c5b14c5..8156b2b 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -44,7 +44,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) > @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct > cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes) > { > const void *reg_base = plat->regbase; > - void *dest_addr = plat->ahbbase; > + void *dest_addr = plat->trigger_base; This can be const void * > unsigned int retry = CQSPI_REG_RETRY; > unsigned int sram_level; > unsigned int wr_bytes; > @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct > cadence_spi_platdata *plat) > > /* Indirect mode configurations */ > writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION); > - writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK, > + writel((u32)plat->trigger_base, Drop the cast. > plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > > /* Disable all interrupts */ > @@ -753,7 +752,7 @@ int cadence_qspi_apb_indirect_read_execute(struct > cadence_spi_platdata *plat, plat->regbase + CQSPI_REG_INDIRECTRD); > > if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf, > - (const void *)plat->ahbbase, rxlen)) > + (const void *)plat->trigger_base, rxlen)) Drop the cast. > goto failrd; > > /* Check flash indirect controller */
Hi, > -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: Thursday, August 27, 2015 1:40 AM > To: Vikas MANOCHA > Cc: u-boot@lists.denx.de; sr@denx.de; grmoore@opensource.altera.com; > jteki@openedev.com > Subject: Re: [PATCH v5 3/5] spi: cadence_qspi: fix base trigger address & > transfer start address > > On Thursday, August 27, 2015 at 12:44:28 AM, Vikas Manocha wrote: > > This patch is to separate the base trigger from the read/write > > transfer start addresses. > > > > 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->triggerbase is added in device tree for mapped spi flash address. > > > > Signed-off-by: Vikas Manocha <vikas.manocha@st.com> > > --- > > > > Changes in v5: None. > > Changes in v4: > > - fifo-width & trigger address alligned to linux device tree binding. > > - renaming of one parameter moved to separate patch. > > - trigger address of socfpga reverted back to 0x0. > > > > Changes in v3: formatted string breaking fixed. > > Changes in v2: Rebased to master > > > > arch/arm/dts/socfpga.dtsi | 1 + > > arch/arm/dts/stv0991.dts | 1 + > > drivers/spi/cadence_qspi.c | 9 +++++---- > > drivers/spi/cadence_qspi.h | 1 + > > drivers/spi/cadence_qspi_apb.c | 7 +++---- > > 5 files changed, 11 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi > > index 9b12420..fe6ad80 100644 > > --- a/arch/arm/dts/socfpga.dtsi > > +++ b/arch/arm/dts/socfpga.dtsi > > @@ -639,6 +639,7 @@ > > ext-decoder = <0>; /* external decoder */ > > num-cs = <4>; > > fifo-depth = <128>; > > + cdns,trigger-address = <0x0 0x0010>; > > So you coin this DT prop as a touple here, but in Linux it's has a single > element. This will make dealing with the bindings an outright horror. Nothing is being coined, it is just left here from previous implementation, will remove it. > > > sram-size = <128>; > > bus-num = <2>; > > status = "disabled"; > > diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts index > > fa3fd64..192d9e5 100644 > > --- a/arch/arm/dts/stv0991.dts > > +++ b/arch/arm/dts/stv0991.dts > > @@ -33,6 +33,7 @@ > > <0x40000000 0x1000000>; > > clocks = <3750000>; > > sram-size = <256>; > > + cdns,trigger-address = <0x40000000 0x0000010>; > > status = "okay"; > > > > flash0: n25q32@0 { > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > index 34a0f46..0d1abc8 100644 > > --- a/drivers/spi/cadence_qspi.c > > +++ b/drivers/spi/cadence_qspi.c > > @@ -290,6 +290,8 @@ static int cadence_spi_ofdata_to_platdata(struct > > udevice *bus) > > > > plat->regbase = (void *)data[0]; > > plat->ahbbase = (void *)data[2]; > > + plat->trigger_base = (u32 *)fdtdec_get_addr(blob, node, > > + "cdns,trigger-address"); > > Drop the cast. Trigger base is pointer. > > > /* Use 500KHz as a suitable default */ > > plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", > @@ > > -311,10 +313,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 ahbbase=%p trigger_base=%p max- > frequency=%d > > page-size=%d\n", + __func__, plat->regbase, plat->ahbbase, > > plat->trigger_base, + plat->max_hz, plat->page_size); > > return 0; > > } > > > > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > > index 98e57aa..2f1bd92 100644 > > --- a/drivers/spi/cadence_qspi.h > > +++ b/drivers/spi/cadence_qspi.h > > @@ -18,6 +18,7 @@ struct cadence_spi_platdata { > > unsigned int max_hz; > > void *regbase; > > void *ahbbase; > > + void *trigger_base; > > > > u32 page_size; > > u32 block_size; > > diff --git a/drivers/spi/cadence_qspi_apb.c > > b/drivers/spi/cadence_qspi_apb.c index c5b14c5..8156b2b 100644 > > --- a/drivers/spi/cadence_qspi_apb.c > > +++ b/drivers/spi/cadence_qspi_apb.c > > @@ -44,7 +44,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) > > @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct > > cadence_spi_platdata *plat, const void *src_addr, unsigned int > > num_bytes) { > > const void *reg_base = plat->regbase; > > - void *dest_addr = plat->ahbbase; > > + void *dest_addr = plat->trigger_base; > > This can be const void * Could be but not relevant to this patch. > > > unsigned int retry = CQSPI_REG_RETRY; > > unsigned int sram_level; > > unsigned int wr_bytes; > > @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct > > cadence_spi_platdata *plat) > > > > /* Indirect mode configurations */ > > writel((plat->sram_size/2), plat->regbase + > CQSPI_REG_SRAMPARTITION); > > - writel((u32)plat->ahbbase & > CQSPI_INDIRECTTRIGGER_ADDR_MASK, > > + writel((u32)plat->trigger_base, > > Drop the cast. Trigger base is pointer. > > > plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > > > > /* Disable all interrupts */ > > @@ -753,7 +752,7 @@ int > cadence_qspi_apb_indirect_read_execute(struct > > cadence_spi_platdata *plat, plat->regbase + CQSPI_REG_INDIRECTRD); > > > > if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf, > > - (const void *)plat->ahbbase, rxlen)) > > + (const void *)plat->trigger_base, rxlen)) > > Drop the cast. DITTO Cheers, Vikas > > > goto failrd; > > > > /* Check flash indirect controller */
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index 9b12420..fe6ad80 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -639,6 +639,7 @@ ext-decoder = <0>; /* external decoder */ num-cs = <4>; fifo-depth = <128>; + cdns,trigger-address = <0x0 0x0010>; sram-size = <128>; bus-num = <2>; status = "disabled"; diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts index fa3fd64..192d9e5 100644 --- a/arch/arm/dts/stv0991.dts +++ b/arch/arm/dts/stv0991.dts @@ -33,6 +33,7 @@ <0x40000000 0x1000000>; clocks = <3750000>; sram-size = <256>; + cdns,trigger-address = <0x40000000 0x0000010>; status = "okay"; flash0: n25q32@0 { diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..0d1abc8 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -290,6 +290,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->regbase = (void *)data[0]; plat->ahbbase = (void *)data[2]; + plat->trigger_base = (u32 *)fdtdec_get_addr(blob, node, + "cdns,trigger-address"); /* Use 500KHz as a suitable default */ plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", @@ -311,10 +313,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 ahbbase=%p trigger_base=%p max-frequency=%d page-size=%d\n", + __func__, plat->regbase, plat->ahbbase, plat->trigger_base, + plat->max_hz, plat->page_size); return 0; } diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 98e57aa..2f1bd92 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -18,6 +18,7 @@ struct cadence_spi_platdata { unsigned int max_hz; void *regbase; void *ahbbase; + void *trigger_base; u32 page_size; u32 block_size; diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index c5b14c5..8156b2b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -44,7 +44,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) @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes) { const void *reg_base = plat->regbase; - void *dest_addr = plat->ahbbase; + void *dest_addr = plat->trigger_base; unsigned int retry = CQSPI_REG_RETRY; unsigned int sram_level; unsigned int wr_bytes; @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat) /* Indirect mode configurations */ writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION); - writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK, + writel((u32)plat->trigger_base, plat->regbase + CQSPI_REG_INDIRECTTRIGGER); /* Disable all interrupts */ @@ -753,7 +752,7 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, plat->regbase + CQSPI_REG_INDIRECTRD); if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf, - (const void *)plat->ahbbase, rxlen)) + (const void *)plat->trigger_base, rxlen)) goto failrd; /* Check flash indirect controller */
This patch is to separate the base trigger from the read/write transfer start addresses. 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->triggerbase is added in device tree for mapped spi flash address. Signed-off-by: Vikas Manocha <vikas.manocha@st.com> --- Changes in v5: None. Changes in v4: - fifo-width & trigger address alligned to linux device tree binding. - renaming of one parameter moved to separate patch. - trigger address of socfpga reverted back to 0x0. Changes in v3: formatted string breaking fixed. Changes in v2: Rebased to master arch/arm/dts/socfpga.dtsi | 1 + arch/arm/dts/stv0991.dts | 1 + drivers/spi/cadence_qspi.c | 9 +++++---- drivers/spi/cadence_qspi.h | 1 + drivers/spi/cadence_qspi_apb.c | 7 +++---- 5 files changed, 11 insertions(+), 8 deletions(-)