[v2,09/10] ASoC: dt-bindings: mt8183: add ec-codec
diff mbox series

Message ID 20191005164320.09.Iec97a3f137148cdf316056612590b3e0b302f5f3@changeid
State Superseded
Headers show
Series
  • ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: support WoV
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Tzung-Bi Shih Oct. 5, 2019, 8:55 a.m. UTC
Add an optional property "ec-codec".  If specified, mt8183 could use the
"wake on voice" feature offered by EC codec.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 .../bindings/sound/mt8183-mt6358-ts3a227-max98357.txt          | 3 +++
 1 file changed, 3 insertions(+)

Comments

Rob Herring Oct. 11, 2019, 3:20 p.m. UTC | #1
On Sat, Oct 05, 2019 at 04:55:08PM +0800, Tzung-Bi Shih wrote:
> Add an optional property "ec-codec".  If specified, mt8183 could use the
> "wake on voice" feature offered by EC codec.
> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
>  .../bindings/sound/mt8183-mt6358-ts3a227-max98357.txt          | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> index 17ff3892f439..decaa013a07e 100644
> --- a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> +++ b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> @@ -6,12 +6,15 @@ Required properties:
>  
>  Optional properties:
>  - mediatek,headset-codec: the phandles of ts3a227 codecs
> +- mediatek,ec-codec: the phandle of EC codecs.
> +                     See google,cros-ec-codec.txt for more details.

Not the best designed audio binding here. We really should just have 
links to codecs and then you can look at the codec nodes to determine 
the type.

>  
>  Example:
>  
>  	sound {
>  		compatible = "mediatek,mt8183_mt6358_ts3a227_max98357";

Don't you need to add EC codec to this? Just kidding. Just highlighting 
the weirdness of this binding.

>  		mediatek,headset-codec = <&ts3a227>;
> +		mediatek,ec-codec = <&ec_codec>;
>  		mediatek,platform = <&afe>;
>  	};
>  
> -- 
> 2.23.0.581.g78d2f28ef7-goog
>
Tzung-Bi Shih Oct. 12, 2019, 3:15 p.m. UTC | #2
On Fri, Oct 11, 2019 at 11:20 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Oct 05, 2019 at 04:55:08PM +0800, Tzung-Bi Shih wrote:
> > Add an optional property "ec-codec".  If specified, mt8183 could use the
> > "wake on voice" feature offered by EC codec.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> > ---
> >  .../bindings/sound/mt8183-mt6358-ts3a227-max98357.txt          | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> > index 17ff3892f439..decaa013a07e 100644
> > --- a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> > +++ b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
> > @@ -6,12 +6,15 @@ Required properties:
> >
> >  Optional properties:
> >  - mediatek,headset-codec: the phandles of ts3a227 codecs
> > +- mediatek,ec-codec: the phandle of EC codecs.
> > +                     See google,cros-ec-codec.txt for more details.
>
> Not the best designed audio binding here. We really should just have
> links to codecs and then you can look at the codec nodes to determine
> the type.

Did you mean: we should use an "audio-codec" array.  In the machine
driver, we should maintain a table of correspondence of compatible
string and the related context.  And use of_device_is_compatible( ) to
determine their types?  Something similar to
https://elixir.bootlin.com/linux/v5.3.5/source/sound/soc/rockchip/rk3399_gru_sound.c#L436

>
> >
> >  Example:
> >
> >       sound {
> >               compatible = "mediatek,mt8183_mt6358_ts3a227_max98357";
>
> Don't you need to add EC codec to this? Just kidding. Just highlighting
> the weirdness of this binding.

Could you explain some?  I cannot understand the "weird" here.  I
thought add the property "mediatek,ec-codec" could be enough.
Or did you mean: the compatible string should reflect the EC codec presence?

>
> >               mediatek,headset-codec = <&ts3a227>;
> > +             mediatek,ec-codec = <&ec_codec>;
> >               mediatek,platform = <&afe>;
> >       };
> >
> > --
> > 2.23.0.581.g78d2f28ef7-goog
> >

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
index 17ff3892f439..decaa013a07e 100644
--- a/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
+++ b/Documentation/devicetree/bindings/sound/mt8183-mt6358-ts3a227-max98357.txt
@@ -6,12 +6,15 @@  Required properties:
 
 Optional properties:
 - mediatek,headset-codec: the phandles of ts3a227 codecs
+- mediatek,ec-codec: the phandle of EC codecs.
+                     See google,cros-ec-codec.txt for more details.
 
 Example:
 
 	sound {
 		compatible = "mediatek,mt8183_mt6358_ts3a227_max98357";
 		mediatek,headset-codec = <&ts3a227>;
+		mediatek,ec-codec = <&ec_codec>;
 		mediatek,platform = <&afe>;
 	};