Re: [PATCH v2 1/2] spi: fsl-spi: Fix parameter ram offset setup for CPM1

From: leroy christophe
Date: Wed Oct 08 2014 - 12:22:03 EST



Le 07/10/2014 02:15, Scott Wood a Ãcrit :
On Sat, 2014-10-04 at 14:02 +0200, christophe leroy wrote:
Le 03/10/2014 22:29, Scott Wood a Ãcrit :
On Fri, 2014-10-03 at 18:49 +0200, Christophe Leroy wrote:
On CPM1, the SPI parameter RAM has a default location. In fsl_spi_cpm_get_pram()
there was a confusion between the SPI_BASE register and the base of the SPI
parameter RAM. Fortunatly, it was working properly with MPC866 and MPC885
because they do set SPI_BASE, but on MPC860 and other old MPC8xx that doesn't
set SPI_BASE, pram_ofs was not properly set. This patch fixes this confusion.

Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>

---
Changes from v1 to v2: none

drivers/spi/spi-fsl-cpm.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..0f3a912 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -262,15 +262,14 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
out_be16(spi_base, pram_ofs);
} else {
- struct spi_pram __iomem *pram = spi_base;
- u16 rpbase = in_be16(&pram->rpbase);
+ u16 rpbase = in_be16(spi_base);
- /* Microcode relocation patch applied? */
+ /* Microcode relocation patch applied | rpbase set by default */
if (rpbase) {
pram_ofs = rpbase;
} else {
- pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
- out_be16(spi_base, pram_ofs);
+ pram_ofs = offsetof(cpm8xx_t, cp_dparam[PROFF_SPI]) -
+ offsetof(cpm8xx_t, cp_dpmem[0]);
}
}
Why is PROFF_SPI not coming from the device tree?
That's where it starts to become tricky.

PROFF_SPI is defined in cpm1.h which is included by the driver already.
Yes, but those values shouldn't be used. It's a leftover from the old
way of hardcoding things and describing the hardware with kconfig rather
than the device tree.

It provides the default offset from the start of the parameter RAM.
Previously I had the following in my device tree, and the last part of
the source above (the one for rpbase == 0) could not work.

spi: spi@a80 {
cell-index = <0>;
compatible = "fsl,spi", "fsl,cpm1-spi";
reg = <0xa80 0x30 0x3d80 0x30>;

First reg area was the area for SPI registers. Second area was the
parameter RAM zone, which was just mapped to get access to the SPI_BASE
pointer (rpbase)

Now I have

compatible = "fsl,spi", "fsl,cpm1-spi-reloc";
reg = <0xa80 0x30 0x3dac 0x2>;

First reg area is the area for SPI registers. Second area is the
SPI_BASE, as for the CPM2.

On recent 8xx (885 and 866 at least) it contains the offset (=0x1D80) of
the parameter RAM. But on old ones (860, ...) it contains 0. Therefore
we have to get the default index in another way.
What I wanted was to keep something similar to what's done with CPM2.

What should it look like if that offset had to be in the device tree ?
If the offset is not relocatable or discoverable, it should stay in the
device tree. If you have an old chip you wouldn't have
fsl,cpm1-spi-reloc and thus you'd still have "0x3d80 0x30" in reg.
This index is from the start of the dual port RAM. It is 0x2000 above the start of the CPM area.
In the DTS, we have:

soc@ff000000 {
compatible = "fsl,mpc885", "fsl,pq1-soc";
#address-cells = <1>;
#size-cells = <1>;
device_type = "soc";
ranges = <0x0 0xff000000 0x28000>;
bus-frequency = <0>;
clock-frequency = <0>;

cpm@9c0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,mpc885-cpm", "fsl,cpm1";
ranges;
reg = <0x9c0 0x40>;
brg-frequency = <0>;
interrupts = <0>; // cpm error interrupt
interrupt-parent = <&CPM_PIC>;

muram@2000 {
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x2000 0x2000>;

data@0 {
compatible = "fsl,cpm-muram-data";
reg = <0x0 0x1c00>;
};
};

spi: spi@a80 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl,spi", "fsl,cpm1-spi";
reg = <0xa80 0x30 0x3d80 0x30>;
interrupts = <5>;
interrupt-parent = <&CPM_PIC>;
mode = "cpu";


The binding allows me to do an of_iomap() on the parameter RAM, hence to get access to the relocation index which is inside it.
But if the relocation index is 0, I have to calculate it by myself because the calling function expects it in return.
The binding is also supposed to tell that the muram is at 0xff002000. But I don't know how I can get this info and use it to calculate the index of my param RAM ? I need to calculate the index which is 1d80 (0x3d80 - 0x2000)

Christophe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/