diff mbox

[V7,02/10] ASoC: qcom: Document LPASS CPU bindings

Message ID 1425428519-12224-3-git-send-email-kwestfie@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kenneth Westfield March 4, 2015, 12:21 a.m. UTC
From: Kenneth Westfield <kwestfie@codeaurora.org>

Add documentation to the sound directory of the
device-tree bindings for the QTi LPASS CPU DAI
device.

Signed-off-by: Kenneth Westfield <kwestfie@codeaurora.org>
Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
---
 .../devicetree/bindings/sound/qcom,lpass-cpu.txt   | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt

Comments

Mark Brown March 5, 2015, 5:38 p.m. UTC | #1
On Tue, Mar 03, 2015 at 04:21:51PM -0800, Kenneth Westfield wrote:
> From: Kenneth Westfield <kwestfie@codeaurora.org>
> 
> Add documentation to the sound directory of the
> device-tree bindings for the QTi LPASS CPU DAI
> device.

Applied, thanks.
Kumar Gala March 5, 2015, 6:52 p.m. UTC | #2
On Mar 3, 2015, at 6:21 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:

> From: Kenneth Westfield <kwestfie@codeaurora.org>
> 
> Add documentation to the sound directory of the
> device-tree bindings for the QTi LPASS CPU DAI
> device.
> 
> Signed-off-by: Kenneth Westfield <kwestfie@codeaurora.org>
> Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
> ---
> .../devicetree/bindings/sound/qcom,lpass-cpu.txt   | 49 ++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> new file mode 100644
> index 0000000000000000000000000000000000000000..e7c6e9321863f022ebf0d51b75d7bb83c10b9062
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> @@ -0,0 +1,49 @@
> +* Qualcomm Technologies LPASS CPU DAI
> +
> +This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
> +
> +Required properties:
> +
> +- compatible		: "qcom,lpass-cpu"
> +- clocks		: Must contain an entry for each entry in clock-names.
> +- clock-names		: A list which must include the following entries:
> +				* "ahbix-clk"
> +				* "mi2s-osr-clk"
> +				* "mi2s-bit-clk"
> +- interrupts		: Must contain an entry for each entry in
> +			  interrupt-names.
> +- interrupt-names	: A list which must include the following entries:
> +				* "lpass-irq-lpaif"
> +- pinctrl-N		: One property must exist for each entry in
> +			  pinctrl-names.  See ../pinctrl/pinctrl-bindings.txt
> +			  for details of the property values.
> +- pinctrl-names		: Must contain a "default" entry.
> +- reg			: Must contain an address for each entry in reg-names.
> +- reg-names		: A list which must include the following entries:
> +				* "lpass-lpaif"
> +
> +Required subnodes:
> +
> +- qcom,adsp		: Audio DSP sub-node
> +

What is the intent of this subnode?

> +Optional Audio DSP subnode properties:
> +
> +- status		: "disabled" indicates the adsp is not available.
> +
> +Example:
> +
> +lpass@28100000 {
> +	compatible = "qcom,lpass-cpu";
> +	clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
> +	clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
> +	interrupts = <0 85 1>;
> +	interrupt-names = "lpass-irq-lpaif";
> +	pinctrl-names = "default", "idle";
> +	pinctrl-0 = <&mi2s_default>;
> +	pinctrl-1 = <&mi2s_idle>;
> +	reg = <0x28100000 0x10000>;
> +	reg-names = "lpass-lpaif";
> +	qcom,adsp {
> +		status = "disabled";
> +	};
> +};
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kenneth Westfield March 6, 2015, 1:51 a.m. UTC | #3
On Thu, Mar 05, 2015 at 12:52:30PM -0600, Kumar Gala wrote:
> 
> On Mar 3, 2015, at 6:21 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
> 
> > +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> > @@ -0,0 +1,49 @@
> > +* Qualcomm Technologies LPASS CPU DAI
> > +
> > +Required subnodes:
> > +
> > +- qcom,adsp		: Audio DSP sub-node
> > +
> > +Optional Audio DSP subnode properties:
> > +
> > +- status		: "disabled" indicates the adsp is not available.
> > +
> 
> What is the intent of this subnode?
> 

From the cover letter:
Even though the ipq806x LPASS does not contain an audio DSP, other SOCs
do have one.  For those SOCs, the audio DSP typically controls the
hardware blocks in the LPASS.  Hence, different CPU DAI driver(s) would
need to be used in order to facilitate audio with the DSP.  As such, the
LPASS DT contains an adsp subnode, which is disabled for this SOC.  The
same subnode should be enabled and populated for other SOCs that do
contain an audio DSP.  Not using the audio DSP would require different
CPU DAI driver(s), in addition to possible bootloader and/or firmware
changes.

This was the result of a request from Mark.  See here:
http://thread.gmane.org/gmane.linux.drivers.devicetree/109331/focus=11633
Kumar Gala March 6, 2015, 4:07 p.m. UTC | #4
On Mar 5, 2015, at 7:51 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:

> On Thu, Mar 05, 2015 at 12:52:30PM -0600, Kumar Gala wrote:
>> 
>> On Mar 3, 2015, at 6:21 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
>> 
>>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>> @@ -0,0 +1,49 @@
>>> +* Qualcomm Technologies LPASS CPU DAI
>>> +
>>> +Required subnodes:
>>> +
>>> +- qcom,adsp		: Audio DSP sub-node
>>> +
>>> +Optional Audio DSP subnode properties:
>>> +
>>> +- status		: "disabled" indicates the adsp is not available.
>>> +
>> 
>> What is the intent of this subnode?
>> 
> 
> From the cover letter:
> Even though the ipq806x LPASS does not contain an audio DSP, other SOCs
> do have one.  For those SOCs, the audio DSP typically controls the
> hardware blocks in the LPASS.  Hence, different CPU DAI driver(s) would
> need to be used in order to facilitate audio with the DSP.  As such, the
> LPASS DT contains an adsp subnode, which is disabled for this SOC.  The
> same subnode should be enabled and populated for other SOCs that do
> contain an audio DSP.  Not using the audio DSP would require different
> CPU DAI driver(s), in addition to possible bootloader and/or firmware
> changes.
> 
> This was the result of a request from Mark.  See here:
> http://thread.gmane.org/gmane.linux.drivers.devicetree/109331/focus=11633

Two quick comments before I read Mark’s comments.

1. Its not normal practice to put something into a DT that does not exist.  Having a node, but marking it disabled implies existence.
2. How would one normally address the audio DSP if it did exist.  I’m just wondering if having a subnode is the proper solution vs maybe a phandle

- k
Kenneth Westfield March 6, 2015, 10:03 p.m. UTC | #5
On Fri, Mar 06, 2015 at 10:07:01AM -0600, Kumar Gala wrote:
> On Mar 5, 2015, at 7:51 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
> > On Thu, Mar 05, 2015 at 12:52:30PM -0600, Kumar Gala wrote:
> >> On Mar 3, 2015, at 6:21 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
> >> 
> >>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> >>> @@ -0,0 +1,49 @@
> >>> +* Qualcomm Technologies LPASS CPU DAI
> >>> +
> >>> +Required subnodes:
> >>> +
> >>> +- qcom,adsp		: Audio DSP sub-node
> >>> +
> >>> +Optional Audio DSP subnode properties:
> >>> +
> >>> +- status		: "disabled" indicates the adsp is not available.
> >>> +
> >> 
> >> What is the intent of this subnode?
> >> 
> > 
> > From the cover letter:
> > Even though the ipq806x LPASS does not contain an audio DSP, other SOCs
> > do have one.  For those SOCs, the audio DSP typically controls the
> > hardware blocks in the LPASS.  Hence, different CPU DAI driver(s) would
> > need to be used in order to facilitate audio with the DSP.  As such, the
> > LPASS DT contains an adsp subnode, which is disabled for this SOC.  The
> > same subnode should be enabled and populated for other SOCs that do
> > contain an audio DSP.  Not using the audio DSP would require different
> > CPU DAI driver(s), in addition to possible bootloader and/or firmware
> > changes.
> > 
> > This was the result of a request from Mark.  See here:
> > http://thread.gmane.org/gmane.linux.drivers.devicetree/109331/focus=11633
> 
> Two quick comments before I read Mark?s comments.
> 
> 1. Its not normal practice to put something into a DT that does not exist.  Having a node, but marking it disabled implies existence.

Will change the DT definition to optional.

> 2. How would one normally address the audio DSP if it did exist.  I?m just wondering if having a subnode is the proper solution vs maybe a phandle

The audio DSP is, in fact, contained within the audio subsystem.  The
representation of that relationship in the DT, I believe, would be a subnode.
OTOH, if there is a strong sentiment towards using a phandle, that would be
fine with me.
Kumar Gala March 6, 2015, 10:06 p.m. UTC | #6
On Mar 6, 2015, at 4:03 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:

> On Fri, Mar 06, 2015 at 10:07:01AM -0600, Kumar Gala wrote:
>> On Mar 5, 2015, at 7:51 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
>>> On Thu, Mar 05, 2015 at 12:52:30PM -0600, Kumar Gala wrote:
>>>> On Mar 3, 2015, at 6:21 PM, Kenneth Westfield <kwestfie@codeaurora.org> wrote:
>>>> 
>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>>> @@ -0,0 +1,49 @@
>>>>> +* Qualcomm Technologies LPASS CPU DAI
>>>>> +
>>>>> +Required subnodes:
>>>>> +
>>>>> +- qcom,adsp		: Audio DSP sub-node
>>>>> +
>>>>> +Optional Audio DSP subnode properties:
>>>>> +
>>>>> +- status		: "disabled" indicates the adsp is not available.
>>>>> +
>>>> 
>>>> What is the intent of this subnode?
>>>> 
>>> 
>>> From the cover letter:
>>> Even though the ipq806x LPASS does not contain an audio DSP, other SOCs
>>> do have one.  For those SOCs, the audio DSP typically controls the
>>> hardware blocks in the LPASS.  Hence, different CPU DAI driver(s) would
>>> need to be used in order to facilitate audio with the DSP.  As such, the
>>> LPASS DT contains an adsp subnode, which is disabled for this SOC.  The
>>> same subnode should be enabled and populated for other SOCs that do
>>> contain an audio DSP.  Not using the audio DSP would require different
>>> CPU DAI driver(s), in addition to possible bootloader and/or firmware
>>> changes.
>>> 
>>> This was the result of a request from Mark.  See here:
>>> http://thread.gmane.org/gmane.linux.drivers.devicetree/109331/focus=11633
>> 
>> Two quick comments before I read Mark?s comments.
>> 
>> 1. Its not normal practice to put something into a DT that does not exist.  Having a node, but marking it disabled implies existence.
> 
> Will change the DT definition to optional.
> 
>> 2. How would one normally address the audio DSP if it did exist.  I?m just wondering if having a subnode is the proper solution vs maybe a phandle
> 
> The audio DSP is, in fact, contained within the audio subsystem.  The
> representation of that relationship in the DT, I believe, would be a subnode.
> OTOH, if there is a strong sentiment towards using a phandle, that would be
> fine with me.

Just depends on how we communicate with the DSP.  If its mostly via MMIO access than a sub node makes sense.  If its via some other RPC/communication mechanism than possibly a phandle.  Trying to understand a bit more to than see what I’d recommend.

- k
Mark Brown March 7, 2015, 11:18 a.m. UTC | #7
On Fri, Mar 06, 2015 at 04:06:48PM -0600, Kumar Gala wrote:

As previously and repeatedly requested please fix your mailer to word
wrap within paragraphs, not doing this makes your mails harder to read
and reply to.  I've reflowed your message for legibility.

> > The audio DSP is, in fact, contained within the audio subsystem.  The
> > representation of that relationship in the DT, I believe, would be a subnode.
> > OTOH, if there is a strong sentiment towards using a phandle, that would be
> > fine with me.

> Just depends on how we communicate with the DSP.  If its mostly via
> MMIO access than a sub node makes sense.  If its via some other
> RPC/communication mechanism than possibly a phandle.  Trying to
> understand a bit more to than see what I’d recommend.

It should be a phandle, while these things are all part of the same
logical function in the CPU they're separate IPs and there may be many
DAIs.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
new file mode 100644
index 0000000000000000000000000000000000000000..e7c6e9321863f022ebf0d51b75d7bb83c10b9062
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
@@ -0,0 +1,49 @@ 
+* Qualcomm Technologies LPASS CPU DAI
+
+This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
+
+Required properties:
+
+- compatible		: "qcom,lpass-cpu"
+- clocks		: Must contain an entry for each entry in clock-names.
+- clock-names		: A list which must include the following entries:
+				* "ahbix-clk"
+				* "mi2s-osr-clk"
+				* "mi2s-bit-clk"
+- interrupts		: Must contain an entry for each entry in
+			  interrupt-names.
+- interrupt-names	: A list which must include the following entries:
+				* "lpass-irq-lpaif"
+- pinctrl-N		: One property must exist for each entry in
+			  pinctrl-names.  See ../pinctrl/pinctrl-bindings.txt
+			  for details of the property values.
+- pinctrl-names		: Must contain a "default" entry.
+- reg			: Must contain an address for each entry in reg-names.
+- reg-names		: A list which must include the following entries:
+				* "lpass-lpaif"
+
+Required subnodes:
+
+- qcom,adsp		: Audio DSP sub-node
+
+Optional Audio DSP subnode properties:
+
+- status		: "disabled" indicates the adsp is not available.
+
+Example:
+
+lpass@28100000 {
+	compatible = "qcom,lpass-cpu";
+	clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
+	clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
+	interrupts = <0 85 1>;
+	interrupt-names = "lpass-irq-lpaif";
+	pinctrl-names = "default", "idle";
+	pinctrl-0 = <&mi2s_default>;
+	pinctrl-1 = <&mi2s_idle>;
+	reg = <0x28100000 0x10000>;
+	reg-names = "lpass-lpaif";
+	qcom,adsp {
+		status = "disabled";
+	};
+};