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? -- 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