From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1946463AbbHGXJq (ORCPT ); Fri, 7 Aug 2015 19:09:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47429 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1946320AbbHGXJp (ORCPT ); Fri, 7 Aug 2015 19:09:45 -0400 Subject: Re: [PATCH v2] staging: ion: Add a default struct device for cma heap To: Greg Kroah-Hartman , Feng Tang References: <1438919413-14440-1-git-send-email-feng.tang@intel.com> <20150807045404.GA26925@kroah.com> <20150807064601.GA9338@shbuild888> <20150807155004.GA22612@shbuild888> <20150807180546.GD10282@kroah.com> Cc: Michal Nazarewicz , John Stultz , Andrew Morton , Kyungmin Park , Marek Szyprowski , Joonsoo Kim , LKML , Sumit Semwal From: Laura Abbott Message-ID: <55C53A98.7030403@redhat.com> Date: Fri, 7 Aug 2015 16:09:12 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20150807180546.GD10282@kroah.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/07/2015 11:05 AM, Greg Kroah-Hartman wrote: > On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote: >> On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote: >>> On Fri, Aug 07 2015, Feng Tang wrote: >>>> As I described above, the dummy struct device is only needed for >>>> dma request, its lifetime is align with the cma_heap itself. >>> >>> Again, this is from perspective of someone who is unfamiliar with ION, >>> but perhaps a viable solution is to bypass DMA API and just call >>> cma_alloc directly? >> >> For ion cma heap, the buffer allocation func ion_cma_allocate() will >> call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is >> implemented by each architeture(arm/m68k/x86 etc), and many Arch's >> implementation doesn't use cma, but use alloc_pages() like APIs. >> So I'm afraid we can't direcly call cma_alloc directly here. > > Ick. But using a "fake" struct device here, for no real reason, > makes me very nervous that you are going to hit a codepath somewhere > that assumes this is a "real" struct device and tries to do something > with it (dev_printk(), look up what bus it is on, change the name of it, > etc.) Trying to fake out the subsystem in this manner is a sign that > something is really wrong here. > > Please either make this a real device, or fix up the api to not need > this type of thing. > I think this issue represents one of the many current issues with Ion. When the void * == struct dev was added, everything was working off of board files. We now have devicetree which makes the device association even more awkward to pull off. Every vendor out there is doing something different right now so the assertion in the commit text about 'normal' is not true; existing code has managed to work with the (not super great) API. There is going to be an Ion session at Plumbers in a few weeks. I'd like to propose holding off on merging anything until after plumbers when there can be some more discussion about what would be a reasonable API, taking into consideration the points brought up in this patch series. Thanks, Laura