From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753035AbcHLSqI (ORCPT ); Fri, 12 Aug 2016 14:46:08 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:36429 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751804AbcHLSqD (ORCPT ); Fri, 12 Aug 2016 14:46:03 -0400 Subject: Re: [PATCH 1/2] remoteproc: core: Add rproc OF look-up functions To: Bjorn Andersson References: <20160810174049.GO26240@tuxbot> <7650e5fa-f52c-1ab5-e197-8630fd7d0322@ti.com> <20160810204049.GV26240@tuxbot> <20160810211947.GW26240@tuxbot> <20160811073122.GA1715@dell> <05d4c9c1-79ea-bde3-e5e5-d26e6b07c916@ti.com> <06f795ef-e71c-2cda-5306-5b75e0f15125@ti.com> <20160812180751.GH26240@tuxbot> CC: Lee Jones , Tony Lindgren , "ohad@wizery.com" , "kernel@stlinux.com" , "linux-remoteproc@vger.kernel.org" , "patrice.chotard@st.com" , "linux-kernel@vger.kernel.org" , "ludovic.barre@st.com" , "ssantosh@kernel.org" , "linux-arm-kernel@lists.infradead.org" , Dave Gerlach From: Suman Anna Message-ID: Date: Fri, 12 Aug 2016 13:45:22 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160812180751.GH26240@tuxbot> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/12/2016 01:07 PM, Bjorn Andersson wrote: > On Fri 12 Aug 09:37 PDT 2016, Suman Anna wrote: > >> Hi Bjorn, >> >>>> On 08/11/2016 02:31 AM, Lee Jones wrote: >>>>> On Wed, 10 Aug 2016, Suman Anna wrote: >>>>> >>>>>> On 08/10/2016 04:19 PM, Bjorn Andersson wrote: >>>>>>> On Wed 10 Aug 14:04 PDT 2016, Suman Anna wrote: >>>>>>> >>>>>>>> On 08/10/2016 03:40 PM, Bjorn Andersson wrote: >>>>>>>>> On Wed 10 Aug 12:37 PDT 2016, Suman Anna wrote: >>>>>>>>> >>>>>>>>>> Hi Lee, Bjorn, >>>>>>>>>> >>>>>>>>>> On 08/10/2016 12:40 PM, Bjorn Andersson wrote: >>>>>>>>>>> On Tue 19 Jul 08:49 PDT 2016, Lee Jones wrote: >>>>>>>>>>> >>>>>>>>>>>> - of_rproc_by_index(): look-up and obtain a reference to a rproc >>>>>>>>>>>> using the DT phandle "rprocs" and a index. >>>>>>>>>>>> >>>>>>>>>>>> - of_rproc_by_name(): lookup and obtain a reference to a rproc >>>>>>>>>>>> using the DT phandle "rprocs" and "rproc-names". >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Ludovic Barre >>>>>>>>>>>> Signed-off-by: Lee Jones >>>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> I'm happy with this, so I whipped up a binding document describing our >>>>>>>>>>> two new properties. Waiting for an opinion on that before I merge this. >> >> One last comment on this is the return code convention change on these >> rproc_get APIs. I am fine in general with returning ERR_PTRs, but most >> of the remoteproc code is using NULL checking for rproc. If you remember >> the discussion back during the hwspinlock DT conversion [1], Ohad >> preferred to return NULL, and that's why even the rproc_get_by_phandle >> was returning NULL. We ought to make this consistent across the board if >> we want to make this switch. >> > > I think it makes sense to return the actual error from these functions, > if nothing else to keep it consistent with other frameworks. > > The other case I see returning NULL is rproc_alloc(), which is think is > analog to kmalloc(), so I think that's fine to keep. > > Luckily wkup_m3 is the only consumer of this API in the kernel today, so > we shouldn't have any issues wrt changing the return value here. Yeah, not worried about the consumers, I am talking about the few functions in remoteproc core code that do some checking like in rproc_del(), __rproc_boot() or rproc_report_crash(). We would need to modify these to use IS_ERR_OR_NULL instead of a NULL check atleast. regards Suman > >> regards >> Suman >> >> [1] http://marc.info/?t=138965891200008 > > Regards, > Bjorn >