Messages in this thread | | | Subject | Re: [PATCH 1/2] remoteproc: core: Add rproc OF look-up functions | From | Suman Anna <> | Date | Fri, 12 Aug 2016 13:45:22 -0500 |
| |
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 <ludovic.barre@st.com> >>>>>>>>>>>> Signed-off-by: Lee Jones <lee.jones@linaro.org> >>>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> 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 >
| |