On 12/05/2016 11:46 AM, Dwivedi, Avaneesh Kumar (avani) wrote: > > > On 11/30/2016 2:52 PM, Stanimir Varbanov wrote: >> Hi, >> >> On 11/30/2016 07:24 AM, Dwivedi, Avaneesh Kumar (avani) wrote: >>> >>> On 11/30/2016 12:57 AM, Stephen Boyd wrote: >>>> On 11/29, Avaneesh Kumar Dwivedi wrote: >>>>> This patch is based on >>>>> https://patchwork.kernel.org/patch/9415627/ >>>>> https://patchwork.kernel.org/patch/9415651/ >>>>> >>>>> This patch add clock initialization, enable and disable support. >>>>> Required resource name string and rating are differentiated based >>>>> on compatible string. Also added documentation for venus pil on >>>>> msm8996. >>>>> >>>>> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@xxxxxxxxxxxxxx> >>>>> --- >>>>> .../devicetree/bindings/remoteproc/qcom,venus.txt | 26 ++++- >>>>> drivers/remoteproc/qcom_venus_pil.c | 116 >>>>> ++++++++++++++++++++- >>>>> 2 files changed, 140 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt >>>>> b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt >>>>> index 2d73ba1..417026b 100644 >>>>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt >>>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt >>>>> @@ -6,13 +6,30 @@ on the Qualcomm Venus remote processor core. >>>>> - compatible: >>>>> Usage: required >>>>> Value type: <string> >>>>> - Definition: must contain "qcom,venus-pil" >>>>> + Definition: must contain "qcom,venus-pil" or >>>>> + "qcom,venus-msm8996-pil" >>>>> - memory-region: >>>>> Usage: required >>>>> Value type: <phandle> >>>>> Definition: a phandle to a node describing reserved memory >>>>> +- clocks: >>>>> + Usage: required >>>>> + Value type: <prop-encoded-array> >>>>> + Definition: reference to the core, iface and bus and maxi clocks >>>>> to be held on >>>>> + behalf of the booting of the venus core >>>>> + >>>>> +- clock-names: >>>>> + Usage: required >>>>> + Value type: <stringlist> >>>>> + Definition: should be "core_clk", "iface_clk", "bus_clk", >>>>> "maxi_clk" >>>> Please drop _clk from all clock names. >>> OK >>>>> + >>>>> +- power-domains: >>>>> + Usage: required >>>>> + Value type: <prop-encoded-array> >>>>> + Definition: reference to the venus gdsc to be turned on before >>>>> booting venus core >>>> All these new properties can't be required if the original >>>> compatible is used, right? >>> gdsc were earlier handled via regulator framework, but now gdsc's are >>> being handled via power domain framework. >>> so any driver which require gdsc to be enabled need to use this property >>> on mainline kernel. >>> so these properties will even be applicable for 8916 venus rproc on >>> mainline kernel. >> Then I'd propose to add compatible string for msm8916 too. We will need >> to distinguish between SoCs in order to select proper Venus firmware >> file too. >> >> I think using power domain in remoteproc driver will break power >> management of the v4l2 venus driver. Presently I use runtime pm to >> switch ON/OFF GDSC and enable/disable Venus clocks. >> >> When the .probe of venus remoteproc driver is called the power domain >> will become ON and never switched OFF (except when platform driver >> .remove method is called). One possible solution would be to add runtime >> pm in venus remoreproc driver. >> >> IMO we should think more about that. >> >> Bjorn, Stephen what are your opinions? >> > Shall I go ahead and add runtime PM API calls during shutdown/start call > along with other changes to address other comments? i was waiting any > comment from Sboyed and Bjorn. > IMO runtime should be added in remoteproc core, so you can start with an RFC patch to see how is going. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html