Date: Tue, 18 Oct 2016 08:38:16 -0500
From: Rob Herring <robh@kernel.org>
To: Markus Mayer <mmayer@broadcom.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        Mark Rutland <mark.rutland@arm.com>,
        Broadcom Kernel List <bcm-kernel-feedback-list@broadcom.com>,
        Device Tree List <devicetree@vger.kernel.org>,
        Power Management List <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 1/3] dt: cpufreq: brcm: New binding document for
 brcmstb-avs-cpufreq
Message-ID: <20161018133816.hcrvy52a67ig4oco@rob-hp-laptop>
References: <1476310376-32699-1-git-send-email-mmayer@broadcom.com>
 <1476310376-32699-2-git-send-email-mmayer@broadcom.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1476310376-32699-2-git-send-email-mmayer@broadcom.com>
User-Agent: Mutt/1.6.2-neo (2016-08-21)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3876
Lines: 109

On Wed, Oct 12, 2016 at 03:12:54PM -0700, Markus Mayer wrote:
> Add the binding document for the new brcmstb-avs-cpufreq driver.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../bindings/cpufreq/brcm,stb-avs-cpu-freq.txt     | 76 ++++++++++++++++++++++
>  MAINTAINERS                                        |  7 ++
>  2 files changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt b/Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
> new file mode 100644
> index 0000000..23a2c3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
> @@ -0,0 +1,76 @@
> +Broadcom STB AVS CPUfreq driver bindings

Bindings describe h/w, not drivers or Linux subsystems.

> +========================================
> +
> +A total of three DT nodes are required. One node (brcm,avs-cpu-data-mem)
> +references the mailbox register used to communicate with the AVS CPU[1]. The
> +second node (brcm,avs-cpu-l2-intr) is required to trigger an interrupt on
> +the AVS CPU. The interrupt tells the AVS CPU that it needs to process a
> +command sent to it by a driver. Interrupting the AVS CPU is mandatory for
> +commands to be processed.
> +
> +The interface also requires a reference to the AVS host interrupt controller,
> +so a driver can react to interrupts generated by the AVS CPU whenever a command
> +has been processed. See [2] for more information on the brcm,l2-intc node.
> +
> +[1] The AVS CPU is an independent co-processor that runs proprietary
> +firmware. On some SoCs, this firmware supports DFS and DVFS in addition to
> +Adaptive Voltage Scaling.
> +
> +[2] Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
> +
> +
> +Node brcm,avs-cpu-data-mem
> +--------------------------
> +
> +Required properties:
> +- compatible: Sould be one of: brcm,avs-cpu-data-mem,
> +              brcm,bcm7271-avs-cpu-data-mem or brcm,bcm7268-avs-cpu-data-mem

This does not match the example. Need to clarify valid combinations and 
ordering.

> +- reg: Specifies base physical address and size of the registers.
> +- interrupts: The interrupt that the AVS CPU will use to interrupt the host
> +              when a command completed.
> +- interrupt-parent: The interrupt controller the above interrupt is routed
> +                    through.
> +- interrupt-names: The name of the interrupt used to interrupt the host.
> +
> +Optional properties:
> +- None
> +
> +Node brcm,avs-cpu-l2-intr
> +-------------------------
> +
> +Required properties:
> +- compatible: Sould be one of: brcm,avs-cpu-l2-intr,
> +              brcm,bcm7271-avs-cpu-l2-intr or brcm,bcm7268-avs-cpu-l2-intr

ditto.

> +- reg: Specifies base physical address and size of the registers.
> +
> +Optional properties:
> +- None
> +
> +
> +Example
> +=======
> +
> +	avs_host_l2_intc: interrupt-controller@f04d1200 {
> +		#interrupt-cells = <1>;
> +		compatible = "brcm,l2-intc";
> +		interrupt-parent = <&intc>;
> +		reg = <0xf04d1200 0x48>;
> +		interrupt-controller;
> +		interrupts = <0x0 0x19 0x0>;
> +		interrupt-names = "avs";
> +	};
> +
> +	avs-cpu-data-mem@f04c4000 {
> +		compatible = "brcm,bcm7271-avs-cpu-data-mem",
> +				"brcm,avs-cpu-data-mem";
> +		reg = <0xf04c4000 0x60>;
> +		interrupts = <0x1a>;
> +		interrupt-parent = <&avs_host_l2_intc>;
> +		interrupt-names = "sw_intr";
> +	};
> +
> +	avs-cpu-l2-intr@f04d1100 {
> +		compatible = "brcm,bcm7271-avs-cpu-l2-intr",
> +				"brcm,avs-cpu-l2-intr";
> +		reg = <0xf04d1100 0x10>;
> +	};

I'm wondering if this would be better described as just 1 node for the 
AVS cpu? Are there any other functions or resources tied to the cpu 
beyond your current usage?

Rob