diff mbox

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

Message ID 1418612114-19692-1-git-send-email-ykk@rock-chips.com (mailing list archive)
State New, archived
Headers show

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.

[..]
Yakir Yang Dec. 15, 2014, 9:28 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.


Yang KuanKuan | Software Engineer | 15060416783 | ykk@rock-chips.com
 
From: Lars-Peter Clausen

Date: 2014-12-15 17:02
To: Yakir Yang
CC: Heiko Stuebner; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Russell King; Philipp Zabel; Zubair.Kakakhel; Fabio Estevam; Shawn Guo; dri-devel; Daniel Kurtz; Sonny Rao; Dylan Reid; Xu Jianqun; St??phane Marchesin; Brian Austin; Bard Liao; Max Filippov; Oder Chiou; devicetree; linux-kernel; linux-arm-kernel; linux-rockchip; alsa-devel; Arnd Bergmann; David Airlie; Sean Cross; Grant Likely; Greg Kroah-Hartman; Ben Zhang; Liam Girdwood; Jaroslav Kysela; Mark Brown; Takashi Iwai; Jyri Sarha
Subject: Re: [PATCH 5/6] dt-bindings: Add documentation for Rockchip hdmi-audio
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.
 
[..]
Yakir Yang Dec. 15, 2014, 9:46 a.m. UTC | #3
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.
>
> [..]
>
>
>
Russell King - ARM Linux Dec. 15, 2014, 10:40 a.m. UTC | #4
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 | #5
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 | #6
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).
Mark Brown Dec. 15, 2014, 4:18 p.m. UTC | #7
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 | #8
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.
Yakir Yang Dec. 16, 2014, 2:38 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.

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.
Yakir Yang Dec. 16, 2014, 6:17 a.m. UTC | #10
? 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.
Russell King - ARM Linux Dec. 16, 2014, 9:34 a.m. UTC | #11
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 | #12
? 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.
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";
+};