cpm: Describe multi-user ram in its own device node.

Hello Scott,

Looks good, only one note:

Quoted 10 lines · On Fri, 28 Sep 2007 14:06:16 -0500 Scott Wood wrote:

arch/powerpc/sysdev/cpm2_common.c +3 -1

+	im_dprambase = cpm2_immr->im_dprambase;
+
 	/* Attach the usable dpmem area */
 	/* XXX: This is actually crap. CPM_DATAONLY_BASE and
 	 * CPM_DATAONLY_SIZE is only a subset of the available dpram. It
 	 * varies with the processor and the microcode patches activated.
 	 * But the following should be at least safe.
 	 */
-	rh_attach_region(&cpm_dpmem_info, 0, r.end - r.start + 1);
+	rh_attach_region(&cpm_dpmem_info, CPM_MAP_ADDR + CPM_DATAONLY_BASE,

arch/powerpc/sysdev/cpm2_common.c static void cpm2_dpinit(void) · line 353

Quoted 3 lines

arch/powerpc/sysdev/cpm2_common.c +1 -0

+	                 CPM_DATAONLY_SIZE);
 }

Can we have something to address upper comment? I mean,any way to have dpram beginning and size encoded in the device tree? We seem to be adding new bus, and still pulling the information from the defines. Maybe I miss something here, but it looks a bit odd.

Quoted 17 lines · Vitaly Bordug

Hello Scott,

Looks good, only one note:

Quoted 10 lines · On Fri, 28 Sep 2007 14:06:16 -0500 Scott Wood wrote:

arch/powerpc/sysdev/cpm2_common.c +3 -1

+	im_dprambase = cpm2_immr->im_dprambase;
+
 	/* Attach the usable dpmem area */
 	/* XXX: This is actually crap. CPM_DATAONLY_BASE and
 	 * CPM_DATAONLY_SIZE is only a subset of the available dpram. It
 	 * varies with the processor and the microcode patches activated.
 	 * But the following should be at least safe.
 	 */
-	rh_attach_region(&cpm_dpmem_info, 0, r.end - r.start + 1);
+	rh_attach_region(&cpm_dpmem_info, CPM_MAP_ADDR + CPM_DATAONLY_BASE,

Quoted 8 lines

arch/powerpc/sysdev/cpm2_common.c static void cpm2_dpinit(void) · line 353

Quoted 3 lines

arch/powerpc/sysdev/cpm2_common.c +1 -0

+	                 CPM_DATAONLY_SIZE);
 }

Can we have something to address upper comment? I mean,any way to have dpram beginning and size encoded in the device tree? We seem to be adding new bus, and still pulling the information from the defines. Maybe I miss something here, but it looks a bit odd.

This bit is #ifndef CONFIG_PPC_CPM_NEW_BINDING (and can come out once all arch/powerpc boards are converted and tested -- I think it's just mpc866ads and CPM mpc85xx left to go). The new code in arch/powerpc/sysdev/cpm_common.c does get it from the device tree.

The way the current CPM binding describes available multi-user (a.k.a. dual-ported) RAM doesn't work well when there are multiple free regions, and it doesn't work at all if the region doesn't begin at the start of the muram area (as the hardware needs to be programmed with offsets into this area). The latter situation can happen with SMC UARTs on CPM2, as its parameter RAM is relocatable, u-boot puts it at zero, and the kernel doesn't support moving it.

It is now described with a muram node, similar to QE. The current CPM binding is sufficiently recent (i.e. never appeared in an official release) that compatibility with existing device trees is not an issue.

The code supporting the new binding is shared between cpm1 and cpm2, rather than remain separated. QE should be able to use this code as well, once minor fixes are made to its device trees.

Signed-off-by Scott Wood <[email protected]>

---
 Documentation/powerpc/booting-without-of.txt |   40 ++++++-
 arch/powerpc/Kconfig.debug                   |    6 +-
 arch/powerpc/boot/cpm-serial.c               |   44 +++++--
 arch/powerpc/boot/dts/ep88xc.dts             |   13 ++-
 arch/powerpc/boot/dts/mpc8272ads.dts         |   11 ++
 arch/powerpc/boot/dts/mpc885ads.dts          |   13 ++-
 arch/powerpc/boot/dts/pq2fads.dts            |   13 ++-
 arch/powerpc/sysdev/commproc.c               |   11 ++-
 arch/powerpc/sysdev/cpm2_common.c            |   36 ++----
 arch/powerpc/sysdev/cpm_common.c             |  159 ++++++++++++++++++++++++++
 drivers/serial/cpm_uart/cpm_uart_cpm2.c      |    4 +-
 include/asm-powerpc/commproc.h               |   12 ++
 include/asm-powerpc/cpm.h                    |   14 +++
 include/asm-powerpc/cpm2.h                   |   10 ++
 14 files changed, 338 insertions(+), 48 deletions(-)

Kumar,

Realizing this may suffer a bit from cleanest-dts flame war, but anyway I pretty much see a lot of sense in getting this in during next merge window. Is this possible?

Quoted 5 lines · Vitaly Bordug

Kumar,

Realizing this may suffer a bit from cleanest-dts flame war, but anyway I pretty much see a lot of
sense in getting this in during next merge window. Is this possible?

Probably, does QE have muram? I can't keep these things straight.

Hello Kumar,

Quoted 5 lines · On Fri, 28 Sep 2007 18:53:38 -0500 Kumar Gala wrote:

Quoted 2 lines · Vitaly Bordug

Kumar,

Quoted 6 lines

Quoted 3 lines

Realizing this may suffer a bit from cleanest-dts flame war, but anyway I pretty much see a lot of
sense in getting this in during next merge window. Is this possible?

Probably, does QE have muram? I can't keep these things straight.

cpms have dpram, qe has muram. these two are the same stuff in fact. Or you are asking about have QE stuff utilize such a binding at the same pass?

Quoted 1 line · Vitaly Bordug

cpms have dpram, qe has muram. these two are the same stuff in fact. Or you are asking about have QE stuff utilize such a binding at the same pass?

I was asking about both these things.

Quoted 3 lines · Kumar Gala

Quoted 1 line · Vitaly Bordug

cpms have dpram, qe has muram. these two are the same stuff in fact. Or you are asking about have QE stuff utilize such a binding at the same pass?

I was asking about both these things.

As stated in the commit message, QE can use this; it just needs a compatible entry in the data node.

Quoted 6 lines · Scott Wood

Quoted 3 lines · Kumar Gala

Quoted 1 line · Vitaly Bordug

cpms have dpram, qe has muram. these two are the same stuff in fact. Or you are asking about have QE stuff utilize such a binding at the same pass?

I was asking about both these things.

As stated in the commit message, QE can use this; it just needs a compatible entry in the data node.

can some one look at that.

The code to process this node is qe_muram_init() in arch/powerpc/sysdev/qe_lib/qe.c.

    if ((np = of_find_node_by_name(NULL, "data-only")) != NULL) {
        address = *of_get_address(np, 0, &size, &flags);
        of_node_put(np);
        rh_attach_region(&qe_muram_info,
            (void *)address, (int)size);
    }

I think it would be trivial to modify this code to look for a Scott-style muram node. Heck, it could be modified to look for both, and so we'll maintain compatibility.

I was thinking of just removing the muram code from qe_lib, and having it use the code in cpm_common.c.

BTW, searching the entire device tree for a node with the name "data-only", regardless of context, seems a bit insane to me.

Quoted 1 line · Scott Wood

I was thinking of just removing the muram code from qe_lib, and having it use the code in cpm_common.c.

Do we want to have the QE library include CPM code at this point? I know we want to merge some QE and CPM code, but I would rather do that in one shot than piecemeal.

Quoted 1 line · Scott Wood

I was thinking of just removing the muram code from qe_lib, and having it use the code in cpm_common.c.

Do we want to have the QE library include CPM code at this point? I know we want to merge some QE and CPM code, but I would rather do that in one shot than piecemeal.

Quoted 1 line

BTW, searching the entire device tree for a node with the name "data-only", regardless of context, seems a bit insane to me.

Yes, I noticed that, too.

The way the current CPM binding describes available multi-user (a.k.a. dual-ported) RAM doesn't work well when there are multiple free regions, and it doesn't work at all if the region doesn't begin at the start of the muram area (as the hardware needs to be programmed with offsets into this area). The latter situation can happen with SMC UARTs on CPM2, as its parameter RAM is relocatable, u-boot puts it at zero, and the kernel doesn't support moving it.

It is now described with a muram node, similar to QE. The current CPM binding is sufficiently recent (i.e. never appeared in an official release) that compatibility with existing device trees is not an issue.

The code supporting the new binding is shared between cpm1 and cpm2, rather than remain separated. QE should be able to use this code as well, once minor fixes are made to its device trees.

Signed-off-by Scott Wood <[email protected]>