On Fri, 2015-10-23 at 11:00 AM, Wood Scott-B07421 [off-list ref] wrote:

-----Original Message-----

From: Wood Scott-B07421

Sent: Friday, October 23, 2015 11:00 AM

To: Zhao Qiang-B45475 <redacted>

Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;

lauraa@codeaurora.org; Xie Xiaobo-R63061 [off-list ref];

benh@kernel.crashing.org; Li Yang-Leo-R58472 [off-list ref];

paulus@samba.org

Subject: Re: [PATCH v12 3/6] CPM/QE: use genalloc to manage CPM/QE muram

On Wed, 2015-10-14 at 15:16 +0800, Zhao Qiang wrote:

quoted

Use genalloc to manage CPM/QE muram instead of rheap.

quoted

quoted

Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com<mailto:qiang.zhao@freescale.com>>

quoted

---

quoted

Changes for v9:

quoted

      - splitted from patch 3/5, modify cpm muram management functions.

quoted

Changes for v10:

quoted

      - modify cpm muram first, then move to qe_common

quoted

      - modify commit.

quoted

Changes for v11:

quoted

      - factor out the common alloc code

quoted

      - modify min_alloc_order to zero for cpm_muram_alloc_fixed.

quoted

Changes for v12:

quoted

      - Nil

quoted

quoted

 arch/powerpc/include/asm/cpm.h   |   1 +

quoted

 arch/powerpc/platforms/Kconfig   |   2 +-

quoted

 arch/powerpc/sysdev/cpm_common.c | 129

quoted

+++++++++++++++++++++++++++---------

quoted

---

quoted

 3 files changed, 93 insertions(+), 39 deletions(-)

quoted

quoted

diff --git a/arch/powerpc/include/asm/cpm.h

quoted

b/arch/powerpc/include/asm/cpm.h index 4398a6c..0e1ac3f 100644

quoted

--- a/arch/powerpc/include/asm/cpm.h

quoted

+++ b/arch/powerpc/include/asm/cpm.h

quoted

@@ -161,6 +161,7 @@ int cpm_muram_init(void);  unsigned long

quoted

cpm_muram_alloc(unsigned long size, unsigned long align);  int

quoted

cpm_muram_free(unsigned long offset);  unsigned long

quoted

cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);

quoted

+unsigned long cpm_muram_alloc_common(unsigned long size, void *data);

quoted

 void __iomem *cpm_muram_addr(unsigned long offset);  unsigned long

quoted

cpm_muram_offset(void __iomem *addr);  dma_addr_t

cpm_muram_dma(void

quoted

__iomem *addr); diff --git a/arch/powerpc/platforms/Kconfig

quoted

b/arch/powerpc/platforms/Kconfig index b7f9c40..01626be7 100644

quoted

--- a/arch/powerpc/platforms/Kconfig

quoted

+++ b/arch/powerpc/platforms/Kconfig

quoted

@@ -275,7 +275,7 @@ config TAU_AVERAGE  config QUICC_ENGINE

quoted

      bool "Freescale QUICC Engine (QE) Support"

quoted

      depends on FSL_SOC && PPC32

quoted

-     select PPC_LIB_RHEAP

quoted

+     select GENERIC_ALLOCATOR

quoted

      select CRC32

quoted

      help

quoted

        The QUICC Engine (QE) is a new generation of communications

quoted

diff --git a/arch/powerpc/sysdev/cpm_common.c

quoted

b/arch/powerpc/sysdev/cpm_common.c

quoted

index 4f78695..ff47072 100644

quoted

--- a/arch/powerpc/sysdev/cpm_common.c

quoted

+++ b/arch/powerpc/sysdev/cpm_common.c

quoted

@@ -17,6 +17,7 @@

quoted

  * published by the Free Software Foundation.

quoted

  */

quoted

quoted

+#include <linux/genalloc.h>

quoted

 #include <linux/init.h>

quoted

 #include <linux/of_device.h>

quoted

 #include <linux/spinlock.h>

quoted

@@ -27,7 +28,6 @@

quoted

quoted

 #include <asm/udbg.h>

quoted

 #include <asm/io.h>

quoted

-#include <asm/rheap.h>

quoted

 #include <asm/cpm.h>

quoted

quoted

 #include <mm/mmu_decl.h>

quoted

@@ -65,14 +65,22 @@ void __init udbg_init_cpm(void)  }  #endif

quoted

quoted

+static struct gen_pool *muram_pool;

quoted

 static spinlock_t cpm_muram_lock;

quoted

-static rh_block_t cpm_boot_muram_rh_block[16]; -static rh_info_t

quoted

cpm_muram_info;  static u8 __iomem *muram_vbase;  static phys_addr_t

quoted

muram_pbase;

quoted

quoted

-/* Max address size we deal with */

quoted

+struct muram_block {

quoted

+     struct list_head head;

quoted

+     unsigned long start;

quoted

+     int size;

quoted

+};

quoted

+

quoted

+static LIST_HEAD(muram_block_list);

quoted

+

quoted

+/* max address size we deal with */

quoted

 #define OF_MAX_ADDR_CELLS    4

quoted

+#define GENPOOL_OFFSET               (4096 * 8)

quoted

quoted

 int cpm_muram_init(void)

quoted

 {

quoted

@@ -87,50 +95,52 @@ int cpm_muram_init(void)

quoted

              return 0;

quoted

quoted

      spin_lock_init(&cpm_muram_lock);

quoted

-     /* initialize the info header */

quoted

-     rh_init(&cpm_muram_info, 1,

quoted

-             sizeof(cpm_boot_muram_rh_block) /

quoted

-             sizeof(cpm_boot_muram_rh_block[0]),

quoted

-             cpm_boot_muram_rh_block);

quoted

-

quoted

      np = of_find_compatible_node(NULL, NULL, "fsl,cpm-muram-data");

quoted

      if (!np) {

quoted

              /* try legacy bindings */

quoted

              np = of_find_node_by_name(NULL, "data-only");

quoted

              if (!np) {

quoted

-                     printk(KERN_ERR "Cannot find CPM muram data node");

quoted

+                     pr_err("Cannot find CPM muram data node");

quoted

                      ret = -ENODEV;

quoted

-                     goto out;

quoted

+                     goto out_muram;

quoted

              }

quoted

      }

quoted

quoted

+     muram_pool = gen_pool_create(0, -1);

quoted

      muram_pbase = of_translate_address(np, zero);

quoted

      if (muram_pbase == (phys_addr_t)OF_BAD_ADDR) {

quoted

-             printk(KERN_ERR "Cannot translate zero through CPM muram node");

quoted

+             pr_err("Cannot translate zero through CPM muram node");

quoted

              ret = -ENODEV;

quoted

-             goto out;

quoted

+             goto out_pool;

quoted

      }

quoted

quoted

      while (of_address_to_resource(np, i++, &r) == 0) {

quoted

              if (r.end > max)

quoted

                      max = r.end;

quoted

+             ret = gen_pool_add(muram_pool, r.start - muram_pbase +

quoted

+                                GENPOOL_OFFSET, resource_size(&r), -1);

quoted

+             if (ret) {

quoted

+                             pr_err("QE: couldn't add muram to pool!\n");

quoted

+                             goto out_pool;

quoted

+                     }

quoted

Whitespace

quoted

-             rh_attach_region(&cpm_muram_info, r.start - muram_pbase,

quoted

-                              resource_size(&r));

quoted

      }

quoted

quoted

      muram_vbase = ioremap(muram_pbase, max - muram_pbase + 1);

quoted

      if (!muram_vbase) {

quoted

-             printk(KERN_ERR "Cannot map CPM muram");

quoted

+             pr_err("Cannot map QE muram");

quoted

              ret = -ENOMEM;

quoted

+             goto out_pool;

quoted

      }

quoted

-

quoted

-out:

quoted

+     goto out_muram;

quoted

+out_pool:

quoted

+     gen_pool_destroy(muram_pool);

quoted

+out_muram:

quoted

      of_node_put(np);

quoted

      return ret;

quoted

 }

quoted

quoted

-/**

quoted

+/*

quoted

  * cpm_muram_alloc - allocate the requested size worth of multi-user ram

quoted

  * @size: number of bytes to allocate

quoted

  * @align: requested alignment, in bytes @@ -141,59 +151,102 @@ out:

quoted

  */

quoted

 unsigned long cpm_muram_alloc(unsigned long size, unsigned long

quoted

align)  {

quoted

-     unsigned long start;

quoted

      unsigned long flags;

quoted

-

quoted

+     unsigned long start;

quoted

+     static struct genpool_data_align muram_pool_data;

quoted

      spin_lock_irqsave(&cpm_muram_lock, flags);

quoted

-     cpm_muram_info.alignment = align;

quoted

-     start = rh_alloc(&cpm_muram_info, size, "commproc");

quoted

-     memset(cpm_muram_addr(start), 0, size);

quoted

+     muram_pool_data.align = align;

quoted

+     gen_pool_set_algo(muram_pool, gen_pool_first_fit_align,

quoted

+                       &muram_pool_data);

quoted

+     start = cpm_muram_alloc_common(size, &muram_pool_data);

quoted

      spin_unlock_irqrestore(&cpm_muram_lock, flags);

quoted

-

quoted

      return start;

quoted

 }

quoted

 EXPORT_SYMBOL(cpm_muram_alloc);

Why is muram_pool_data static?  Why is it being passed to

gen_pool_set_algo()?

Cpm_muram use both align algo and fixed algo, so we need to set corresponding algo and

Algo data.

The whole reason we're adding gen_pool_alloc_data()

is to avoid that.  Do we need gen_pool_alloc_algo() too?

We add gen_pool_alloc_data() to pass data to algo, because align algo and fixed algo,

Because align and fixed algos need specific data.

Also, please maintain a blank line between variable declarations and code.

quoted

+     return (unsigned long) -ENOMEM;

No space after casts.

-Scott