diff mbox

[5/6] dt-bindings: Add documentation for Rockchip hdmi-audio

Message ID 1418612114-19692-1-git-send-email-ykk@rock-chips.com
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Yakir Yang Dec. 15, 2014, 2:55 a.m. UTC 
Required properties:
- compatible: platform specific
- cpu-of-node: the device node of cpu_dai
- codec-name: the dw-hdmi codec's device name
- codec-dai-name: the dw-hdmi codec's dai name

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---

 .../bindings/sound/rockchip,rockchip-hdmi-audio.txt      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rockchip-hdmi-audio.txt

Comments

Lars-Peter Clausen Dec. 15, 2014, 9:02 a.m. UTC | #1 
On 12/15/2014 03:55 AM, Yakir Yang wrote:
[...]
> +- codec-name: the dw-hdmi codec's device name
> +- codec-dai-name: the dw-hdmi codec's dai name

Please you phandles for this, the names are Linux driver and framework 
specific details. That should not leak into the DT bindings.

[..]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yakir Yang Dec. 15, 2014, 9:46 a.m. UTC | #2 
Hi lars:

thks for your review, I will move it to hdmi audio driver, give the 
codec_name & codec_dai_name an const string.

Best Regards.

在 2014年12月15日 17:02, Lars-Peter Clausen 写道:
> On 12/15/2014 03:55 AM, Yakir Yang wrote:
> [...]
>> +- codec-name: the dw-hdmi codec's device name
>> +- codec-dai-name: the dw-hdmi codec's dai name
>
> Please you phandles for this, the names are Linux driver and framework 
> specific details. That should not leak into the DT bindings.
>
> [..]
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Dec. 15, 2014, 10:40 a.m. UTC | #3 
On Mon, Dec 15, 2014 at 10:55:14AM +0800, Yakir Yang wrote:
> Required properties:
> - compatible: platform specific
> - cpu-of-node: the device node of cpu_dai
> - codec-name: the dw-hdmi codec's device name
> - codec-dai-name: the dw-hdmi codec's dai name

This is not actually a separate physical device from the main dw-hdmi
device.  What you are doing is merely describing the Linux kernel
internals in DT, which is not what DT is supposed to be about.  DT is
supposed to describe the hardware in an operating system independent
manner.

Including details like this (because ASoC needs a separate DT node) is
the wrong approach.
Mark Brown Dec. 15, 2014, 12:02 p.m. UTC | #4 
On Mon, Dec 15, 2014 at 10:40:29AM +0000, Russell King - ARM Linux wrote:

> Including details like this (because ASoC needs a separate DT node) is
> the wrong approach.

And indeed there should be no Linux-internal reason for that - we should
be able to use whatever DT node makes sense, if there's some reason
that's not possible we should fix that (we do have several drivers doing
this sort of thing already though).
Yakir Yang Dec. 15, 2014, 1:10 p.m. UTC | #5 
Hi Mark & Russell:

thanks for your replay, what your say is right, those dt is merely 
describing the Linux kernel internals.
Instead of transmit cpu-of-node & codec-name & codec-dai-name through 
dt, i can give those an const
name to rockchip-hdmi-audio.c .

In that way, dt will only need compatible for creating sound device. is 
it ok  ?
sound {
     compatible = "rockchip,rk3288-hdmi-audio";
     status = "okay";
}
could you give me some advises.


Best Regards.

在 2014年12月15日 20:02, Mark Brown 写道:
> On Mon, Dec 15, 2014 at 10:40:29AM +0000, Russell King - ARM Linux wrote:
>
>> Including details like this (because ASoC needs a separate DT node) is
>> the wrong approach.
> And indeed there should be no Linux-internal reason for that - we should
> be able to use whatever DT node makes sense, if there's some reason
> that's not possible we should fix that (we do have several drivers doing
> this sort of thing already though).


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 15, 2014, 4:18 p.m. UTC | #6 
On Mon, Dec 15, 2014 at 09:10:26PM +0800, Kuankuan.Yang wrote:
> Hi Mark & Russell:

Please don't top post, that way people have some context for what you're
saying - look at how people normally format their mails on thelist.

> In that way, dt will only need compatible for creating sound device. is it
> ok  ?
> sound {
>     compatible = "rockchip,rk3288-hdmi-audio";
>     status = "okay";
> }
> could you give me some advises.

That'd be good, though the other question is if this can be made to use
simple-card so we can avoid having a custom driver for this.
Yakir Yang Dec. 16, 2014, 2:14 a.m. UTC | #7 
Hi Mark :

Got it, it will be good to use simple-card. i will try it in next version.

Best Regards.

在 2014年12月16日 00:18, Mark Brown 写道:
> On Mon, Dec 15, 2014 at 09:10:26PM +0800, Kuankuan.Yang wrote:
>> Hi Mark & Russell:
> Please don't top post, that way people have some context for what you're
> saying - look at how people normally format their mails on thelist.
>
>> In that way, dt will only need compatible for creating sound device. is it
>> ok  ?
>> sound {
>>      compatible = "rockchip,rk3288-hdmi-audio";
>>      status = "okay";
>> }
>> could you give me some advises.
> That'd be good, though the other question is if this can be made to use
> simple-card so we can avoid having a custom driver for this.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yakir Yang Dec. 16, 2014, 2:38 a.m. UTC | #8 
在 2014年12月16日 00:18, Mark Brown 写道:
> On Mon, Dec 15, 2014 at 09:10:26PM +0800, Kuankuan.Yang wrote:
>> Hi Mark & Russell:
> Please don't top post, that way people have some context for what you're
> saying - look at how people normally format their mails on thelist.

got it,  thanks for your remind.

>> In that way, dt will only need compatible for creating sound device. is it
>> ok  ?
>> sound {
>>      compatible = "rockchip,rk3288-hdmi-audio";
>>      status = "okay";
>> }
>> could you give me some advises.
> That'd be good, though the other question is if this can be made to use
> simple-card so we can avoid having a custom driver for this.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yakir Yang Dec. 16, 2014, 6:17 a.m. UTC | #9 
在 2014年12月16日 00:18, Mark Brown 写道:
> On Mon, Dec 15, 2014 at 09:10:26PM +0800, Kuankuan.Yang wrote:
>> Hi Mark & Russell:
> Please don't top post, that way people have some context for what you're
> saying - look at how people normally format their mails on thelist.
>
>> In that way, dt will only need compatible for creating sound device. is it
>> ok  ?
>> sound {
>>      compatible = "rockchip,rk3288-hdmi-audio";
>>      status = "okay";
>> }
>> could you give me some advises.
> That'd be good, though the other question is if this can be made to use
> simple-card so we can avoid having a custom driver for this.
Hi Russell  & Mark:

hdmi-audio driver share irq with dw_hdmi driver,  for continuing 
AHBAUDDMA or for detecting hdmi insert.
So maybe it's hard to switch to simple-card way,  Russell what's your 
opinion ?

Best Regards.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Dec. 16, 2014, 9:34 a.m. UTC | #10 
On Tue, Dec 16, 2014 at 02:17:51PM +0800, Kuankuan.Yang wrote:
> hdmi-audio driver share irq with dw_hdmi driver,  for continuing AHBAUDDMA
> or for detecting hdmi insert.
> So maybe it's hard to switch to simple-card way,  Russell what's your
> opinion ?

My "opinion" is that I need to spend some time looking in depth at both
your code and my code before I can form an idea of how to proceed.

However, since I seem to be spending a lot of time chasing regressions
which have appeared in v3.18, I don't have a lot of time to look at this
right now.
Yakir Yang Dec. 16, 2014, 9:48 a.m. UTC | #11 
在 2014年12月16日 17:34, Russell King - ARM Linux 写道:
> On Tue, Dec 16, 2014 at 02:17:51PM +0800, Kuankuan.Yang wrote:
>> hdmi-audio driver share irq with dw_hdmi driver,  for continuing AHBAUDDMA
>> or for detecting hdmi insert.
>> So maybe it's hard to switch to simple-card way,  Russell what's your
>> opinion ?
> My "opinion" is that I need to spend some time looking in depth at both
> your code and my code before I can form an idea of how to proceed.
>
> However, since I seem to be spending a lot of time chasing regressions
> which have appeared in v3.18, I don't have a lot of time to look at this
> right now.
>
okay, if u got any ideas, wish u can let me know, thks.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/rockchip,rockchip-hdmi-audio.txt b/Documentation/devicetree/bindings/sound/rockchip,rockchip-hdmi-audio.txt
new file mode 100644
index 0000000..674cff0
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip,rockchip-hdmi-audio.txt
@@ -0,0 +1,16 @@ 
+Rockchip hdmi audio bindings
+
+Required properties:
+- compatible: platform specific
+- cpu-of-node: the device node of cpu_dai
+- codec-name: the dw-hdmi codec's device name
+- codec-dai-name: the dw-hdmi codec's dai name
+
+Example:
+
+sound {
+	compatible = "rockchip,rk3288-hdmi-audio";
+	cpu-of-node = <&i2s>;
+	codec-name = "dw-hdmi-audio";
+	codec-dai-name = "dw-hdmi-hifi";
+};