<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:宋体;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@宋体";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri",sans-serif;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 124.95pt 1.0in 124.95pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoPlainText">On Fri, 2015-10-23 at 11:00 AM, Wood Scott-B07421 <scottwood@freescale.com> wrote:<o:p></o:p></p>
<p class="MsoPlainText">> -----Original Message-----</p>
<p class="MsoPlainText">> From: Wood Scott-B07421</p>
<p class="MsoPlainText">> Sent: Friday, October 23, 2015 11:00 AM</p>
<p class="MsoPlainText">> To: Zhao Qiang-B45475 <qiang.zhao@freescale.com></p>
<p class="MsoPlainText">> Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;</p>
<p class="MsoPlainText">> lauraa@codeaurora.org; Xie Xiaobo-R63061 <X.Xie@freescale.com>;</p>
<p class="MsoPlainText">> benh@kernel.crashing.org; Li Yang-Leo-R58472 <LeoLi@freescale.com>;</p>
<p class="MsoPlainText">> paulus@samba.org</p>
<p class="MsoPlainText">> Subject: Re: [PATCH v12 3/6] CPM/QE: use genalloc to manage CPM/QE muram</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> On Wed, 2015-10-14 at 15:16 +0800, Zhao Qiang wrote:</p>
<p class="MsoPlainText">> > Use genalloc to manage CPM/QE muram instead of rheap.</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > Signed-off-by: Zhao Qiang <<a href="mailto:qiang.zhao@freescale.com"><span style="color:windowtext;text-decoration:none">qiang.zhao@freescale.com</span></a>></p>
<p class="MsoPlainText">> > ---</p>
<p class="MsoPlainText">> > Changes for v9:</p>
<p class="MsoPlainText">> > - splitted from patch 3/5, modify cpm muram management functions.</p>
<p class="MsoPlainText">> > Changes for v10:</p>
<p class="MsoPlainText">> > - modify cpm muram first, then move to qe_common</p>
<p class="MsoPlainText">> > - modify commit.</p>
<p class="MsoPlainText">> > Changes for v11:</p>
<p class="MsoPlainText">> > - factor out the common alloc code</p>
<p class="MsoPlainText">> > - modify min_alloc_order to zero for cpm_muram_alloc_fixed.</p>
<p class="MsoPlainText">> > Changes for v12:</p>
<p class="MsoPlainText">> > - Nil</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > arch/powerpc/include/asm/cpm.h | 1 +</p>
<p class="MsoPlainText">> > arch/powerpc/platforms/Kconfig | 2 +-</p>
<p class="MsoPlainText">> > arch/powerpc/sysdev/cpm_common.c | 129</p>
<p class="MsoPlainText">> > +++++++++++++++++++++++++++---------</p>
<p class="MsoPlainText">> > ---</p>
<p class="MsoPlainText">> > 3 files changed, 93 insertions(+), 39 deletions(-)</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > diff --git a/arch/powerpc/include/asm/cpm.h</p>
<p class="MsoPlainText">> > b/arch/powerpc/include/asm/cpm.h index 4398a6c..0e1ac3f 100644</p>
<p class="MsoPlainText">> > --- a/arch/powerpc/include/asm/cpm.h</p>
<p class="MsoPlainText">> > +++ b/arch/powerpc/include/asm/cpm.h</p>
<p class="MsoPlainText">> > @@ -161,6 +161,7 @@ int cpm_muram_init(void); unsigned long</p>
<p class="MsoPlainText">> > cpm_muram_alloc(unsigned long size, unsigned long align); int</p>
<p class="MsoPlainText">> > cpm_muram_free(unsigned long offset); unsigned long</p>
<p class="MsoPlainText">> > cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);</p>
<p class="MsoPlainText">> > +unsigned long cpm_muram_alloc_common(unsigned long size, void *data);</p>
<p class="MsoPlainText">> > void __iomem *cpm_muram_addr(unsigned long offset); unsigned long</p>
<p class="MsoPlainText">> > cpm_muram_offset(void __iomem *addr); dma_addr_t</p>
<p class="MsoPlainText">> cpm_muram_dma(void</p>
<p class="MsoPlainText">> > __iomem *addr); diff --git a/arch/powerpc/platforms/Kconfig</p>
<p class="MsoPlainText">> > b/arch/powerpc/platforms/Kconfig index b7f9c40..01626be7 100644</p>
<p class="MsoPlainText">> > --- a/arch/powerpc/platforms/Kconfig</p>
<p class="MsoPlainText">> > +++ b/arch/powerpc/platforms/Kconfig</p>
<p class="MsoPlainText">> > @@ -275,7 +275,7 @@ config TAU_AVERAGE config QUICC_ENGINE</p>
<p class="MsoPlainText">> > bool "Freescale QUICC Engine (QE) Support"</p>
<p class="MsoPlainText">> > depends on FSL_SOC && PPC32</p>
<p class="MsoPlainText">> > - select PPC_LIB_RHEAP</p>
<p class="MsoPlainText">> > + select GENERIC_ALLOCATOR</p>
<p class="MsoPlainText">> > select CRC32</p>
<p class="MsoPlainText">> > help</p>
<p class="MsoPlainText">> > The QUICC Engine (QE) is a new generation of communications</p>
<p class="MsoPlainText">> > diff --git a/arch/powerpc/sysdev/cpm_common.c</p>
<p class="MsoPlainText">> > b/arch/powerpc/sysdev/cpm_common.c</p>
<p class="MsoPlainText">> > index 4f78695..ff47072 100644</p>
<p class="MsoPlainText">> > --- a/arch/powerpc/sysdev/cpm_common.c</p>
<p class="MsoPlainText">> > +++ b/arch/powerpc/sysdev/cpm_common.c</p>
<p class="MsoPlainText">> > @@ -17,6 +17,7 @@</p>
<p class="MsoPlainText">> > * published by the Free Software Foundation.</p>
<p class="MsoPlainText">> > */</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > +#include <linux/genalloc.h></p>
<p class="MsoPlainText">> > #include <linux/init.h></p>
<p class="MsoPlainText">> > #include <linux/of_device.h></p>
<p class="MsoPlainText">> > #include <linux/spinlock.h></p>
<p class="MsoPlainText">> > @@ -27,7 +28,6 @@</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > #include <asm/udbg.h></p>
<p class="MsoPlainText">> > #include <asm/io.h></p>
<p class="MsoPlainText">> > -#include <asm/rheap.h></p>
<p class="MsoPlainText">> > #include <asm/cpm.h></p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > #include <mm/mmu_decl.h></p>
<p class="MsoPlainText">> > @@ -65,14 +65,22 @@ void __init udbg_init_cpm(void) } #endif</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > +static struct gen_pool *muram_pool;</p>
<p class="MsoPlainText">> > static spinlock_t cpm_muram_lock;</p>
<p class="MsoPlainText">> > -static rh_block_t cpm_boot_muram_rh_block[16]; -static rh_info_t</p>
<p class="MsoPlainText">> > cpm_muram_info; static u8 __iomem *muram_vbase; static phys_addr_t</p>
<p class="MsoPlainText">> > muram_pbase;</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > -/* Max address size we deal with */</p>
<p class="MsoPlainText">> > +struct muram_block {</p>
<p class="MsoPlainText">> > + struct list_head head;</p>
<p class="MsoPlainText">> > + unsigned long start;</p>
<p class="MsoPlainText">> > + int size;</p>
<p class="MsoPlainText">> > +};</p>
<p class="MsoPlainText">> > +</p>
<p class="MsoPlainText">> > +static LIST_HEAD(muram_block_list);</p>
<p class="MsoPlainText">> > +</p>
<p class="MsoPlainText">> > +/* max address size we deal with */</p>
<p class="MsoPlainText">> > #define OF_MAX_ADDR_CELLS 4</p>
<p class="MsoPlainText">> > +#define GENPOOL_OFFSET (4096 * 8)</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > int cpm_muram_init(void)</p>
<p class="MsoPlainText">> > {</p>
<p class="MsoPlainText">> > @@ -87,50 +95,52 @@ int cpm_muram_init(void)</p>
<p class="MsoPlainText">> > return 0;</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > spin_lock_init(&cpm_muram_lock);</p>
<p class="MsoPlainText">> > - /* initialize the info header */</p>
<p class="MsoPlainText">> > - rh_init(&cpm_muram_info, 1,</p>
<p class="MsoPlainText">> > - sizeof(cpm_boot_muram_rh_block) /</p>
<p class="MsoPlainText">> > - sizeof(cpm_boot_muram_rh_block[0]),</p>
<p class="MsoPlainText">> > - cpm_boot_muram_rh_block);</p>
<p class="MsoPlainText">> > -</p>
<p class="MsoPlainText">> > np = of_find_compatible_node(NULL, NULL, "fsl,cpm-muram-data");</p>
<p class="MsoPlainText">> > if (!np) {</p>
<p class="MsoPlainText">> > /* try legacy bindings */</p>
<p class="MsoPlainText">> > np = of_find_node_by_name(NULL, "data-only");</p>
<p class="MsoPlainText">> > if (!np) {</p>
<p class="MsoPlainText">> > - printk(KERN_ERR "Cannot find CPM muram data node");</p>
<p class="MsoPlainText">> > + pr_err("Cannot find CPM muram data node");</p>
<p class="MsoPlainText">> > ret = -ENODEV;</p>
<p class="MsoPlainText">> > - goto out;</p>
<p class="MsoPlainText">> > + goto out_muram;</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > + muram_pool = gen_pool_create(0, -1);</p>
<p class="MsoPlainText">> > muram_pbase = of_translate_address(np, zero);</p>
<p class="MsoPlainText">> > if (muram_pbase == (phys_addr_t)OF_BAD_ADDR) {</p>
<p class="MsoPlainText">> > - printk(KERN_ERR "Cannot translate zero through CPM muram node");</p>
<p class="MsoPlainText">> > + pr_err("Cannot translate zero through CPM muram node");</p>
<p class="MsoPlainText">> > ret = -ENODEV;</p>
<p class="MsoPlainText">> > - goto out;</p>
<p class="MsoPlainText">> > + goto out_pool;</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > while (of_address_to_resource(np, i++, &r) == 0) {</p>
<p class="MsoPlainText">> > if (r.end > max)</p>
<p class="MsoPlainText">> > max = r.end;</p>
<p class="MsoPlainText">> > + ret = gen_pool_add(muram_pool, r.start - muram_pbase +</p>
<p class="MsoPlainText">> > + GENPOOL_OFFSET, resource_size(&r), -1);</p>
<p class="MsoPlainText">> > + if (ret) {</p>
<p class="MsoPlainText">> > + pr_err("QE: couldn't add muram to pool!\n");</p>
<p class="MsoPlainText">> > + goto out_pool;</p>
<p class="MsoPlainText">> > + }</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> Whitespace</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> > - rh_attach_region(&cpm_muram_info, r.start - muram_pbase,</p>
<p class="MsoPlainText">> > - resource_size(&r));</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > muram_vbase = ioremap(muram_pbase, max - muram_pbase + 1);</p>
<p class="MsoPlainText">> > if (!muram_vbase) {</p>
<p class="MsoPlainText">> > - printk(KERN_ERR "Cannot map CPM muram");</p>
<p class="MsoPlainText">> > + pr_err("Cannot map QE muram");</p>
<p class="MsoPlainText">> > ret = -ENOMEM;</p>
<p class="MsoPlainText">> > + goto out_pool;</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> > -</p>
<p class="MsoPlainText">> > -out:</p>
<p class="MsoPlainText">> > + goto out_muram;</p>
<p class="MsoPlainText">> > +out_pool:</p>
<p class="MsoPlainText">> > + gen_pool_destroy(muram_pool);</p>
<p class="MsoPlainText">> > +out_muram:</p>
<p class="MsoPlainText">> > of_node_put(np);</p>
<p class="MsoPlainText">> > return ret;</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> ></p>
<p class="MsoPlainText">> > -/**</p>
<p class="MsoPlainText">> > +/*</p>
<p class="MsoPlainText">> > * cpm_muram_alloc - allocate the requested size worth of multi-user ram</p>
<p class="MsoPlainText">> > * @size: number of bytes to allocate</p>
<p class="MsoPlainText">> > * @align: requested alignment, in bytes @@ -141,59 +151,102 @@ out:</p>
<p class="MsoPlainText">> > */</p>
<p class="MsoPlainText">> > unsigned long cpm_muram_alloc(unsigned long size, unsigned long</p>
<p class="MsoPlainText">> > align) {</p>
<p class="MsoPlainText">> > - unsigned long start;</p>
<p class="MsoPlainText">> > unsigned long flags;</p>
<p class="MsoPlainText">> > -</p>
<p class="MsoPlainText">> > + unsigned long start;</p>
<p class="MsoPlainText">> > + static struct genpool_data_align muram_pool_data;</p>
<p class="MsoPlainText">> > spin_lock_irqsave(&cpm_muram_lock, flags);</p>
<p class="MsoPlainText">> > - cpm_muram_info.alignment = align;</p>
<p class="MsoPlainText">> > - start = rh_alloc(&cpm_muram_info, size, "commproc");</p>
<p class="MsoPlainText">> > - memset(cpm_muram_addr(start), 0, size);</p>
<p class="MsoPlainText">> > + muram_pool_data.align = align;</p>
<p class="MsoPlainText">> > + gen_pool_set_algo(muram_pool, gen_pool_first_fit_align,</p>
<p class="MsoPlainText">> > + &muram_pool_data);</p>
<p class="MsoPlainText">> > + start = cpm_muram_alloc_common(size, &muram_pool_data);</p>
<p class="MsoPlainText">> > spin_unlock_irqrestore(&cpm_muram_lock, flags);</p>
<p class="MsoPlainText">> > -</p>
<p class="MsoPlainText">> > return start;</p>
<p class="MsoPlainText">> > }</p>
<p class="MsoPlainText">> > EXPORT_SYMBOL(cpm_muram_alloc);</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> Why is muram_pool_data static? Why is it being passed to</p>
<p class="MsoPlainText">> gen_pool_set_algo()? <o:p></o:p></p>
<p class="MsoPlainText"><span style="color:black">Cpm_muram use both align algo and fixed algo, so we need to set corresponding algo and
<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="color:black">Algo data.<o:p></o:p></span></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">>The whole reason we're adding gen_pool_alloc_data()</p>
<p class="MsoPlainText">> is to avoid that. Do we need gen_pool_alloc_algo() too?</p>
<p class="MsoPlainText"><span style="color:black"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="color:black">We add gen_pool_alloc_data() to pass data to algo, because align algo and fixed algo,<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="color:black">Because align and fixed algos need specific data.<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="color:black"><o:p> </o:p></span></p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> Also, please maintain a blank line between variable declarations and code.</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> > + return (unsigned long) -ENOMEM;</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> No space after casts.</p>
<p class="MsoPlainText">> </p>
<p class="MsoPlainText">> -Scott</p>
<p class="MsoPlainText"><o:p> </o:p></p>
</div>
</body>
</html>