Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document

[Rob Herring]

On Fri, Aug 19, 2016 at 06:53:19PM +0300, Stanimir Varbanov wrote:
> Add devicetree binding document for Venus remote processor.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/remoteproc/qcom,venus.txt  | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> new file mode 100644
> index 000000000000..06a2db60fa38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Venus Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm Venus remote processor core.

This does not make sense to me. Venus is the video encoder/decoder h/w, 
right? Why is the firmware loader separate from the codec block? Why 
rproc is used? Are there multiple clients? Naming it rproc_venus implies 
there aren't. And why does the firmware loading need 8MB of memory at a 
fixed address?

> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must contain "qcom,venus-pil"
> +
> +- memory-region:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: a phandle to a node describing reserved memory
> +
> +* An example
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		venus_mem: venus@89900000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x89900000 0x0 0x800000>;
> +			alignment = <0x1000>;
> +			no-map;
> +		};
> +	};
> +
> +	rproc_venus@0 {
> +		compatible = "qcom,venus-pil";
> +		memory-region = <&venus_mem>;
> +	};
> \ No newline at end of file
> -- 
> 2.7.4
>