From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752741AbbHFLkf (ORCPT ); Thu, 6 Aug 2015 07:40:35 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:35170 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750758AbbHFLkd convert rfc822-to-8bit (ORCPT ); Thu, 6 Aug 2015 07:40:33 -0400 From: Michal Nazarewicz To: Feng Tang , Greg Kroah-Hartman , John Stultz , Andrew Morton , Kyungmin Park , Marek Szyprowski , Joonsoo Kim , linux-kernel@vger.kernel.org Cc: Feng Tang Subject: Re: [PATCH] staging: ion: Add a default struct device for cma heap In-Reply-To: <1438856698-21381-1-git-send-email-feng.tang@intel.com> Organization: http://mina86.com/ References: <1438856698-21381-1-git-send-email-feng.tang@intel.com> User-Agent: Notmuch/0.19+53~g2e63a09 (http://notmuchmail.org) Emacs/25.0.50.1 (x86_64-unknown-linux-gnu) X-Face: PbkBB1w#)bOqd`iCe"Ds{e+!C7`pkC9a|f)Qo^BMQvy\q5x3?vDQJeN(DS?|-^$uMti[3D*#^_Ts"pU$jBQLq~Ud6iNwAw_r_o_4]|JO?]}P_}Nc&"p#D(ZgUb4uCNPe7~a[DbPG0T~!&c.y$Ur,=N4RT>]dNpd;KFrfMCylc}gc??'U2j,!8%xdD Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACQElEQVQ4jW3TMWvbQBQHcBk1xE6WyALX1069oZBMlq+ouUwpEQQ6uRjttkWP4CmBgGM0BQLBdPFZYPsyFUo6uEtKDQ7oy/U96XR2Ux8ehH/89Z6enqxBcS7Lg81jmSuujrfCZcLI/TYYvbGj+jbgFpHJ/bqQAUISj8iLyu4LuFHJTosxsucO4jSDNE0Hq3hwK/ceQ5sx97b8LcUDsILfk+ovHkOIsMbBfg43VuQ5Ln9YAGCkUdKJoXR9EclFBhixy3EGVz1K6eEkhxCAkeMMnqoAhAKwhoUJkDrCqvbecaYINlFKSRS1i12VKH1XpUd4qxL876EkMcDvHj3s5RBajHHMlA5iK32e0C7VgG0RlzFPvoYHZLRmAC0BmNcBruhkE0KsMsbEc62ZwUJDxWUdMsMhVqovoT96i/DnX/ASvz/6hbCabELLk/6FF/8PNpPCGqcZTGFcBhhAaZZDbQPaAB3+KrWWy2XgbYDNIinkdWAFcCpraDE/knwe5DBqGmgzESl1p2E4MWAz0VUPgYYzmfWb9yS4vCvgsxJriNTHoIBz5YteBvg+VGISQWUqhMiByPIPpygeDBE6elD973xWwKkEiHZAHKjhuPsFnBuArrzxtakRcISv+XMIPl4aGBUJm8Emk7qBYU8IlgNEIpiJhk/No24jHwkKTFHDWfPniR4iw5vJaw2nzSjfq2zffcE/GDjRC2dn0J0XwPAbDL84TvaFCJEU4Oml9pRyEUhR3Cl2t01AoEjRbs0sYugp14/4X5n4pU4EHHnMAAAAAElFTkSuQmCC X-PGP: 50751FF4 X-PGP-FP: AC1F 5F5C D418 88F8 CC84 5858 2060 4012 5075 1FF4 X-Hashcash: 1:20:150806:john.stultz@linaro.org::dmCx1zTIwRsRIMqm:0000000000000000000000000000000000000000Fmd X-Hashcash: 1:20:150806:akpm@linux-foundation.org::KFQq5fRBUA/j8VEI:0000000000000000000000000000000000002cg4 X-Hashcash: 1:20:150806:linux-kernel@vger.kernel.org::oNbZwFhFoeuvNMMb:00000000000000000000000000000000030sU X-Hashcash: 1:20:150806:gregkh@linuxfoundation.org::lfG0+YPwEb3Wta1X:000000000000000000000000000000000004cHt X-Hashcash: 1:20:150806:feng.tang@intel.com::FgglPgNl1Ij/MxT6:0000000000000000000000000000000000000000004FMj X-Hashcash: 1:20:150806:feng.tang@intel.com::dFLzRh7Se/viiTmr:0000000000000000000000000000000000000000007Ly2 X-Hashcash: 1:20:150806:kyungmin.park@samsung.com::psQEoQUadkSDCVdt:00000000000000000000000000000000000074pa X-Hashcash: 1:20:150806:iamjoonsoo.kim@lge.com::zuDpirKuLBBO/CgI:0000000000000000000000000000000000000008Nvf X-Hashcash: 1:20:150806:m.szyprowski@samsung.com::PutAoDcjwQEM9Rd6:0000000000000000000000000000000000000Bb2+ Date: Thu, 06 Aug 2015 13:40:28 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 06 2015, Feng Tang wrote: > When trying to use several cma heaps on our platforms, > we met a memory issue due to that the several cma_heaps > are sharing the same "struct device *". > > As in current code base, the normal cma heap creating > process is, one platform device is created during boot, > and it will sequentially create cma heaps (usually passing > its own struct device * as a parameter) > > For the multiple cma heaps case, there will be one "struct > cma" created for each cma heap, and this "struct cma *" is > saved in dev->cma_area. So the single platform device can't > meet the requirement here. > > So this patch add one default device for a cma heap to avoid > sharing the same "struct device", thus fix the issue. And it > doesn't break existing code by only using that default device > when no "struct device *" is passed in. > > Also, since the cma framework has been cleaned up, this patch > also add a platform data member to pass the "struct cma*" to > ion_cma_heap_create(). > > Signed-off-by: Feng Tang >>From CMA’s point of view: Acked-by: Michal Nazarewicz > --- > drivers/staging/android/ion/ion.h | 4 ++++ > drivers/staging/android/ion/ion_cma_heap.c | 20 +++++++++++++++++--- > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h > index 443db84..e9af17e 100644 > --- a/drivers/staging/android/ion/ion.h > +++ b/drivers/staging/android/ion/ion.h > @@ -45,6 +45,9 @@ struct ion_buffer; > * @size: size of the heap in bytes if applicable > * @align: required alignment in physical memory if applicable > * @priv: private info passed from the board file > + * @priv2: when creating CMA heap, platform device should better also > + * pass the "struct cma *" info, so that the cma buffer request > + * know where to go for the buffer > * > * Provided by the board file. > */ > @@ -56,6 +59,7 @@ struct ion_platform_heap { > size_t size; > ion_phys_addr_t align; > void *priv; > + void *priv2; Why are those void pointers anyway? Perhaps just make them struct device *dev and struct cma *cma? Especially since priv2 is a bit awkward name. > }; > > /** > diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c > index f4211f1..b3e8896 100644 > --- a/drivers/staging/android/ion/ion_cma_heap.c > +++ b/drivers/staging/android/ion/ion_cma_heap.c > @@ -29,6 +29,7 @@ > struct ion_cma_heap { > struct ion_heap heap; > struct device *dev; > + struct device default_dma_dev; > }; > > #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap) > @@ -180,9 +181,22 @@ struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) > return ERR_PTR(-ENOMEM); > > cma_heap->heap.ops = &ion_cma_ops; > - /* get device from private heaps data, later it will be > - * used to make the link with reserved CMA memory */ > - cma_heap->dev = data->priv; > + > + /* > + * data->priv for cma heap is currently supposed to point > + * to a "struct device *" > + */ > + if (data->priv) { > + cma_heap->dev = data->priv; > + } else { > + cma_heap->dev = &cma_heap->default_dma_dev; > + cma_heap->dev->coherent_dma_mask = DMA_BIT_MASK(32); > + cma_heap->dev->dma_mask = &dev->coherent_dma_mask; > + } > + > + /* data->priv2 contains a pointer to struct cma */ > + dev_set_cma_area(cma_heap->dev, data->priv2); Perhaps: + if (data->priv2) + dev_set_cma_area(cma_heap->dev, data->priv2); > + > cma_heap->heap.type = ION_HEAP_TYPE_DMA; > return &cma_heap->heap; > } > -- > 1.7.9.5 > -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +------ooO--(_)--Ooo--