linux-kernel - Re: [PATCH 1/2] remoteproc: core: Add rproc OF look-up functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f9641050-0a3f-11de-fa69-e587034e0527@ti.com>
Date:	Wed, 10 Aug 2016 16:04:04 -0500
From:	Suman Anna <s-anna@...com>
To:	Bjorn Andersson <bjorn.andersson@...aro.org>
CC:	Lee Jones <lee.jones@...aro.org>,
	"ohad@...ery.com" <ohad@...ery.com>,
	"kernel@...inux.com" <kernel@...inux.com>,
	"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
	"patrice.chotard@...com" <patrice.chotard@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"ludovic.barre@...com" <ludovic.barre@...com>,
	"ssantosh@...nel.org" <ssantosh@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 1/2] remoteproc: core: Add rproc OF look-up functions

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@...com>
>>>> Signed-off-by: Lee Jones <lee.jones@...aro.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.
>>
>> Yeah, I like the two new API too, I have just about run into the need
>> for the same on my product trees. We can probably make it less
>> complicated than what the current series is. The wkup_m3_ipc usage was
>> before there was any standardization on the property names, so we went
>> with a ti, prefixed property specific to the wkup_m3_ipc node and a
>> general API that is agnostic of property name. We don't have all the
>> pieces for PM on AM335x/AM437x SoCs on upstream kernel yet, so we should
>> be able to switch over to using the new property as we cannot achieve
>> the desired functionality even though we can boot the wkup_m3.
>>
> 
> Glad to hear you're onboard with dropping the old property name, even if
> it's later.
> 
>> Here's what I propose:
>> 1. Let's not burden the new of_get_rproc_by_index() API with having to
>> fall-back and look for ti,rprocs. The rproc_get_by_phandle() core logic
>> of looking up against the rproc list is self-contained, and the API
>> usage is limited to just the wkup_m3_ipc driver at the moment.
>> 2. Keep the rproc_get_by_phandle API as is but mark it as deprecated.
>> IMHO, the rename of this API to of_get_rproc_by_phandle() in Patch 2 but
>> using a device node pointer as input argument doesn't add any value.
>> 3. Provide a fallback in wkup_m3_ipc driver to look for both rprocs and
>> ti,rproc property, while switching over the node to use rprocs property.
>> 4. We can get rid of the rproc_get_by_phandle() API after the
>> wkup_m3_ipc transition is done to use of_get_rproc_by_index() API.
>>
> 
> I don't fancy the idea of leaving the kernel builds with a deprecation
> warning for some time and I don't feel the cost of carrying those 2
> extra statements is a bigger issue than keeping a deprecated API around.
> And it will be less than the dance we have to do in wkup_m3_ipc.
> 
> So I think that we should merge these patches and if it can be concluded
> that we don't need backwards compatibility with the old DT property we
> can drop the inner conditional in the API.

Yeah, I am fine with dropping the inner ti,rproc processing in the new
of_rproc_get_by_index() API and keeping it clean. What's not clear to me
is why we would still need a get_by_phandle API alongside the two new
API once the wkup_m3_ipc is converted to the new API? Or is it just to
clean up the consumer interface? If latter, I will fixup the wkup_m3_ipc
driver without the need for remoteproc core changes, and switch over to
the new API.

regards
Suman