[RFC,v4,4/8] drm/i2c: tda998x: Add support of a DT graph of ports

Commit Message

Jyri Sarha Sept. 18, 2015, 11:06 a.m. UTC

From: Jean-Francois Moine <[e-mail]>

Two kinds of ports may be declared in a DT graph of ports: video and audio.
This patch accepts the port value from a video port as an alternative
to the video-ports property.
It also accepts audio ports in the case the transmitter is not used as
a slave encoder.
The new file include/sound/tda998x.h prepares to the definition of
a tda998x CODEC.

Signed-off-by: Jean-Francois Moine <[e-mail]>
Signed-off-by: Jyri Sarha <[e-mail]>
---
 .../devicetree/bindings/drm/i2c/tda998x.txt        | 51 ++++++++++++
 drivers/gpu/drm/i2c/tda998x_drv.c                  | 90 +++++++++++++++++++---
 include/sound/tda998x.h                            |  8 ++
 3 files changed, 140 insertions(+), 9 deletions(-)
 create mode 100644 include/sound/tda998x.h

Comments

Rob Herring (Arm) Sept. 21, 2015, 3:19 p.m. UTC | #1

On 09/18/2015 06:06 AM, Jyri Sarha wrote:
> From: Jean-Francois Moine <[e-mail]>
> 
> Two kinds of ports may be declared in a DT graph of ports: video and audio.
> This patch accepts the port value from a video port as an alternative
> to the video-ports property.
> It also accepts audio ports in the case the transmitter is not used as
> a slave encoder.
> The new file include/sound/tda998x.h prepares to the definition of
> a tda998x CODEC.
> 
> Signed-off-by: Jean-Francois Moine <[e-mail]>
> Signed-off-by: Jyri Sarha <[e-mail]>
> ---
>  .../devicetree/bindings/drm/i2c/tda998x.txt        | 51 ++++++++++++
>  drivers/gpu/drm/i2c/tda998x_drv.c                  | 90 +++++++++++++++++++---
>  include/sound/tda998x.h                            |  8 ++
>  3 files changed, 140 insertions(+), 9 deletions(-)
>  create mode 100644 include/sound/tda998x.h
> 
> diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> index e9e4bce..35f6a80 100644
> --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> @@ -16,6 +16,35 @@ Optional properties:
>  
>    - video-ports: 24 bits value which defines how the video controller
>  	output is wired to the TDA998x input - default: <0x230145>
> +	This property is not used when ports are defined.
> +
> +Optional nodes:
> +
> +  - port: up to three ports.
> +	The ports are defined according to [1].
> +
> +    Video port.
> +	There may be only one video port.
> +	This one must contain the following property:
> +
> +	- port-type: must be "rgb"

Why do you need this if there is no other choice? The port number should
define which one is video.

> +
> +	and may contain the optional property:
> +
> +	- reg: 24 bits value which defines how the video controller
> +		output is wired to the TDA998x input (video pins)
> +		When absent, the default value is <0x230145>.

I'm failing to decode how this value works. In any case, I would keep
this as a vendor specific property rather than abusing reg.

> +
> +    Audio ports.
> +	There may be one or two audio ports.
> +	These ones must contain the following properties:
> +
> +	- port-type: must be "i2s" or "spdif"
> +
> +	- reg: 8 bits value which defines how the audio controller
> +		output is wired to the TDA998x input (audio pins)

reg is 32-bits.

> +
> +[1] Documentation/devicetree/bindings/graph.txt
>  
>  Example:
>  
> @@ -26,4 +55,26 @@ Example:
>  		interrupts = <27 2>;		/* falling edge */
>  		pinctrl-0 = <&pmx_camera>;
>  		pinctrl-names = "default";
> +
> +		port@230145 {
> +			port-type = "rgb";
> +			reg = <0x230145>;
> +			hdmi_0: endpoint {
> +				remote-endpoint = <&lcd0_0>;
> +			};
> +		};
> +		port@3 {			/* AP1 = I2S */

Is 3 significant? What happened to 0-2?

> +			port-type = "i2s";
> +			reg = <0x03>;
> +			tda998x_i2s: endpoint {
> +				remote-endpoint = <&audio1_i2s>;
> +			};
> +		};
> +		port@4 {			 /* AP2 = S/PDIF */
> +			port-type = "spdif";
> +			reg = <0x04>;
> +			tda998x_spdif: endpoint {
> +				remote-endpoint = <&audio1_spdif1>;
> +			};
> +		};
>  	};
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 424228b..0952eac 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -27,6 +27,7 @@
>  #include <drm/drm_edid.h>
>  #include <drm/drm_of.h>
>  #include <drm/i2c/tda998x.h>
> +#include <sound/tda998x.h>
>  
>  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
>  
> @@ -47,6 +48,8 @@ struct tda998x_priv {
>  	wait_queue_head_t wq_edid;
>  	volatile int wq_edid_wait;
>  	struct drm_encoder *encoder;
> +
> +	struct tda998x_audio audio;
>  };
>  
>  #define to_tda998x_priv(x)  ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv)
> @@ -774,6 +777,8 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv,
>  			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
>  
>  	priv->params = *p;
> +	priv->audio.port_types[0] = p->audio_format;
> +	priv->audio.ports[0] = p->audio_cfg;
>  }
>  
>  static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)
> @@ -1230,9 +1235,57 @@ static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = {
>  
>  /* I2C driver functions */
>  
> +static int tda998x_parse_ports(struct tda998x_priv *priv,
> +				struct device_node *np)
> +{
> +	struct device_node *of_port;
> +	const char *port_type;
> +	int ret, audio_index, reg, afmt;
> +
> +	audio_index = 0;
> +	for_each_child_of_node(np, of_port) {
> +		if (!of_port->name
> +		 || of_node_cmp(of_port->name, "port") != 0)
> +			continue;
> +		ret = of_property_read_string(of_port, "port-type",
> +					&port_type);
> +		if (ret < 0)
> +			continue;
> +		ret = of_property_read_u32(of_port, "reg", &reg);
> +		if (strcmp(port_type, "rgb") == 0) {
> +			if (!ret) {		/* video reg is optional */
> +				priv->vip_cntrl_0 = reg >> 16;
> +				priv->vip_cntrl_1 = reg >> 8;
> +				priv->vip_cntrl_2 = reg;
> +			}
> +			continue;
> +		}
> +		if (strcmp(port_type, "i2s") == 0)
> +			afmt = AFMT_I2S;
> +		else if (strcmp(port_type, "spdif") == 0)
> +			afmt = AFMT_SPDIF;
> +		else
> +			continue;
> +		if (ret < 0) {
> +			dev_err(&priv->hdmi->dev, "missing reg for %s\n",
> +				port_type);
> +			return ret;
> +		}
> +		if (audio_index >= ARRAY_SIZE(priv->audio.ports)) {
> +			dev_err(&priv->hdmi->dev, "too many audio ports\n");
> +			break;
> +		}
> +		priv->audio.ports[audio_index] = reg;
> +		priv->audio.port_types[audio_index] = afmt;
> +		audio_index++;
> +	}
> +	return 0;
> +}
> +
>  static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  {
>  	struct device_node *np = client->dev.of_node;
> +	struct device_node *of_port;
>  	u32 video;
>  	int rev_lo, rev_hi, ret;
>  	unsigned short cec_addr;
> @@ -1337,15 +1390,34 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  	/* enable EDID read irq: */
>  	reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
>  
> -	if (!np)
> -		return 0;		/* non-DT */
> -
> -	/* get the optional video properties */
> -	ret = of_property_read_u32(np, "video-ports", &video);
> -	if (ret == 0) {
> -		priv->vip_cntrl_0 = video >> 16;
> -		priv->vip_cntrl_1 = video >> 8;
> -		priv->vip_cntrl_2 = video;
> +	/* get the device tree parameters */
> +	if (np) {
> +		of_port = of_get_child_by_name(np, "port");
> +		if (of_port) {				/* graph of ports */
> +			of_node_put(of_port);
> +			ret = tda998x_parse_ports(priv, np);
> +			if (ret < 0)
> +				goto fail;
> +
> +			/* initialize the default audio configuration */
> +			if (priv->audio.ports[0]) {
> +				priv->params.audio_cfg = priv->audio.ports[0];
> +				priv->params.audio_format =
> +						priv->audio.port_types[0];
> +				priv->params.audio_clk_cfg =
> +					priv->params.audio_format ==
> +							AFMT_SPDIF ? 0 : 1;
> +			}
> +		} else {
> +
> +			/* optional video properties */
> +			ret = of_property_read_u32(np, "video-ports", &video);
> +			if (ret == 0) {
> +				priv->vip_cntrl_0 = video >> 16;
> +				priv->vip_cntrl_1 = video >> 8;
> +				priv->vip_cntrl_2 = video;
> +			}
> +		}
>  	}
>  
>  	return 0;
> diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h
> new file mode 100644
> index 0000000..bef1da7
> --- /dev/null
> +++ b/include/sound/tda998x.h
> @@ -0,0 +1,8 @@
> +#ifndef SND_TDA998X_H
> +#define SND_TDA998X_H
> +
> +struct tda998x_audio {
> +	u8 ports[2];			/* AP value */
> +	u8 port_types[2];		/* AFMT_xxx */
> +};
> +#endif
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jean-Francois Moine Sept. 24, 2015, 10:36 a.m. UTC | #2

On Mon, 21 Sep 2015 10:19:18 -0500
Rob Herring <[e-mail]> wrote:

> On 09/18/2015 06:06 AM, Jyri Sarha wrote:
> > From: Jean-Francois Moine <[e-mail]>
> > 
> > Two kinds of ports may be declared in a DT graph of ports: video and audio.
> > This patch accepts the port value from a video port as an alternative
> > to the video-ports property.
> > It also accepts audio ports in the case the transmitter is not used as
> > a slave encoder.
> > The new file include/sound/tda998x.h prepares to the definition of
> > a tda998x CODEC.
> > 
> > Signed-off-by: Jean-Francois Moine <[e-mail]>
> > Signed-off-by: Jyri Sarha <[e-mail]>
> > ---
	[snip]
> > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> > index e9e4bce..35f6a80 100644
> > --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> > @@ -16,6 +16,35 @@ Optional properties:
> >  
> >    - video-ports: 24 bits value which defines how the video controller
> >  	output is wired to the TDA998x input - default: <0x230145>
> > +	This property is not used when ports are defined.
> > +
> > +Optional nodes:
> > +
> > +  - port: up to three ports.
> > +	The ports are defined according to [1].
> > +
> > +    Video port.
> > +	There may be only one video port.
> > +	This one must contain the following property:
> > +
> > +	- port-type: must be "rgb"  
> 
> Why do you need this if there is no other choice? The port number should
> define which one is video.

There is no specific port number.
One of the ports is video and two other ones are audio.

> > +
> > +	and may contain the optional property:
> > +
> > +	- reg: 24 bits value which defines how the video controller
> > +		output is wired to the TDA998x input (video pins)
> > +		When absent, the default value is <0x230145>.  
> 
> I'm failing to decode how this value works. In any case, I would keep
> this as a vendor specific property rather than abusing reg.

This has been explained in
https://lkml.org/lkml/2014/1/20/86

> > +
> > +    Audio ports.
> > +	There may be one or two audio ports.
> > +	These ones must contain the following properties:
> > +
> > +	- port-type: must be "i2s" or "spdif"
> > +
> > +	- reg: 8 bits value which defines how the audio controller
> > +		output is wired to the TDA998x input (audio pins)  
> 
> reg is 32-bits.

This port has only 8 significant bits as explained in
https://lkml.org/lkml/2015/1/7/362

> > +
> > +[1] Documentation/devicetree/bindings/graph.txt
> >  
> >  Example:
> >  
> > @@ -26,4 +55,26 @@ Example:
> >  		interrupts = <27 2>;		/* falling edge */
> >  		pinctrl-0 = <&pmx_camera>;
> >  		pinctrl-names = "default";
> > +
> > +		port@230145 {
> > +			port-type = "rgb";
> > +			reg = <0x230145>;
> > +			hdmi_0: endpoint {
> > +				remote-endpoint = <&lcd0_0>;
> > +			};
> > +		};
> > +		port@3 {			/* AP1 = I2S */  
> 
> Is 3 significant? What happened to 0-2?

The value after @ is the 'reg' value (= value of the port register).

> > +			port-type = "i2s";
> > +			reg = <0x03>;
> > +			tda998x_i2s: endpoint {
> > +				remote-endpoint = <&audio1_i2s>;
> > +			};
> > +		};

Rob Herring (Arm) Sept. 24, 2015, 4:29 p.m. UTC | #3

On Thu, Sep 24, 2015 at 5:36 AM, Jean-Francois Moine <[e-mail]> wrote:
> On Mon, 21 Sep 2015 10:19:18 -0500
> Rob Herring <[e-mail]> wrote:
>
>> On 09/18/2015 06:06 AM, Jyri Sarha wrote:
>> > From: Jean-Francois Moine <[e-mail]>
>> >
>> > Two kinds of ports may be declared in a DT graph of ports: video and audio.
>> > This patch accepts the port value from a video port as an alternative
>> > to the video-ports property.
>> > It also accepts audio ports in the case the transmitter is not used as
>> > a slave encoder.
>> > The new file include/sound/tda998x.h prepares to the definition of
>> > a tda998x CODEC.
>> >
>> > Signed-off-by: Jean-Francois Moine <[e-mail]>
>> > Signed-off-by: Jyri Sarha <[e-mail]>
>> > ---
>         [snip]
>> > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > index e9e4bce..35f6a80 100644
>> > --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > @@ -16,6 +16,35 @@ Optional properties:
>> >
>> >    - video-ports: 24 bits value which defines how the video controller
>> >     output is wired to the TDA998x input - default: <0x230145>
>> > +   This property is not used when ports are defined.
>> > +
>> > +Optional nodes:
>> > +
>> > +  - port: up to three ports.
>> > +   The ports are defined according to [1].
>> > +
>> > +    Video port.
>> > +   There may be only one video port.
>> > +   This one must contain the following property:
>> > +
>> > +   - port-type: must be "rgb"
>>
>> Why do you need this if there is no other choice? The port number should
>> define which one is video.
>
> There is no specific port number.
> One of the ports is video and two other ones are audio.

I saying you should define the port numbering in the binding. Port 0
is video, Port 1 is i2s, etc.

>
>> > +
>> > +   and may contain the optional property:
>> > +
>> > +   - reg: 24 bits value which defines how the video controller
>> > +           output is wired to the TDA998x input (video pins)
>> > +           When absent, the default value is <0x230145>.
>>
>> I'm failing to decode how this value works. In any case, I would keep
>> this as a vendor specific property rather than abusing reg.
>
> This has been explained in
> https://lkml.org/lkml/2014/1/20/86

Okay, so that explains how it works, but still it should not be in reg.

>
>> > +
>> > +    Audio ports.
>> > +   There may be one or two audio ports.
>> > +   These ones must contain the following properties:
>> > +
>> > +   - port-type: must be "i2s" or "spdif"
>> > +
>> > +   - reg: 8 bits value which defines how the audio controller
>> > +           output is wired to the TDA998x input (audio pins)
>>
>> reg is 32-bits.
>
> This port has only 8 significant bits as explained in
> https://lkml.org/lkml/2015/1/7/362

Maybe only 8-bits are used, but the size of the data in the DTB is
32-bits unless you add 8-bit annotation. Again, reg is not appropriate
to use here.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
index e9e4bce..35f6a80 100644
--- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
+++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
@@ -16,6 +16,35 @@  Optional properties:
 
   - video-ports: 24 bits value which defines how the video controller
 	output is wired to the TDA998x input - default: <0x230145>
+	This property is not used when ports are defined.
+
+Optional nodes:
+
+  - port: up to three ports.
+	The ports are defined according to [1].
+
+    Video port.
+	There may be only one video port.
+	This one must contain the following property:
+
+	- port-type: must be "rgb"
+
+	and may contain the optional property:
+
+	- reg: 24 bits value which defines how the video controller
+		output is wired to the TDA998x input (video pins)
+		When absent, the default value is <0x230145>.
+
+    Audio ports.
+	There may be one or two audio ports.
+	These ones must contain the following properties:
+
+	- port-type: must be "i2s" or "spdif"
+
+	- reg: 8 bits value which defines how the audio controller
+		output is wired to the TDA998x input (audio pins)
+
+[1] Documentation/devicetree/bindings/graph.txt
 
 Example:
 
@@ -26,4 +55,26 @@  Example:
 		interrupts = <27 2>;		/* falling edge */
 		pinctrl-0 = <&pmx_camera>;
 		pinctrl-names = "default";
+
+		port@230145 {
+			port-type = "rgb";
+			reg = <0x230145>;
+			hdmi_0: endpoint {
+				remote-endpoint = <&lcd0_0>;
+			};
+		};
+		port@3 {			/* AP1 = I2S */
+			port-type = "i2s";
+			reg = <0x03>;
+			tda998x_i2s: endpoint {
+				remote-endpoint = <&audio1_i2s>;
+			};
+		};
+		port@4 {			 /* AP2 = S/PDIF */
+			port-type = "spdif";
+			reg = <0x04>;
+			tda998x_spdif: endpoint {
+				remote-endpoint = <&audio1_spdif1>;
+			};
+		};
 	};
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 424228b..0952eac 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -27,6 +27,7 @@ 
 #include <drm/drm_edid.h>
 #include <drm/drm_of.h>
 #include <drm/i2c/tda998x.h>
+#include <sound/tda998x.h>
 
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
@@ -47,6 +48,8 @@  struct tda998x_priv {
 	wait_queue_head_t wq_edid;
 	volatile int wq_edid_wait;
 	struct drm_encoder *encoder;
+
+	struct tda998x_audio audio;
 };
 
 #define to_tda998x_priv(x)  ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv)
@@ -774,6 +777,8 @@  static void tda998x_encoder_set_config(struct tda998x_priv *priv,
 			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
 
 	priv->params = *p;
+	priv->audio.port_types[0] = p->audio_format;
+	priv->audio.ports[0] = p->audio_cfg;
 }
 
 static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)
@@ -1230,9 +1235,57 @@  static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = {
 
 /* I2C driver functions */
 
+static int tda998x_parse_ports(struct tda998x_priv *priv,
+				struct device_node *np)
+{
+	struct device_node *of_port;
+	const char *port_type;
+	int ret, audio_index, reg, afmt;
+
+	audio_index = 0;
+	for_each_child_of_node(np, of_port) {
+		if (!of_port->name
+		 || of_node_cmp(of_port->name, "port") != 0)
+			continue;
+		ret = of_property_read_string(of_port, "port-type",
+					&port_type);
+		if (ret < 0)
+			continue;
+		ret = of_property_read_u32(of_port, "reg", &reg);
+		if (strcmp(port_type, "rgb") == 0) {
+			if (!ret) {		/* video reg is optional */
+				priv->vip_cntrl_0 = reg >> 16;
+				priv->vip_cntrl_1 = reg >> 8;
+				priv->vip_cntrl_2 = reg;
+			}
+			continue;
+		}
+		if (strcmp(port_type, "i2s") == 0)
+			afmt = AFMT_I2S;
+		else if (strcmp(port_type, "spdif") == 0)
+			afmt = AFMT_SPDIF;
+		else
+			continue;
+		if (ret < 0) {
+			dev_err(&priv->hdmi->dev, "missing reg for %s\n",
+				port_type);
+			return ret;
+		}
+		if (audio_index >= ARRAY_SIZE(priv->audio.ports)) {
+			dev_err(&priv->hdmi->dev, "too many audio ports\n");
+			break;
+		}
+		priv->audio.ports[audio_index] = reg;
+		priv->audio.port_types[audio_index] = afmt;
+		audio_index++;
+	}
+	return 0;
+}
+
 static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 {
 	struct device_node *np = client->dev.of_node;
+	struct device_node *of_port;
 	u32 video;
 	int rev_lo, rev_hi, ret;
 	unsigned short cec_addr;
@@ -1337,15 +1390,34 @@  static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	/* enable EDID read irq: */
 	reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
 
-	if (!np)
-		return 0;		/* non-DT */
-
-	/* get the optional video properties */
-	ret = of_property_read_u32(np, "video-ports", &video);
-	if (ret == 0) {
-		priv->vip_cntrl_0 = video >> 16;
-		priv->vip_cntrl_1 = video >> 8;
-		priv->vip_cntrl_2 = video;
+	/* get the device tree parameters */
+	if (np) {
+		of_port = of_get_child_by_name(np, "port");
+		if (of_port) {				/* graph of ports */
+			of_node_put(of_port);
+			ret = tda998x_parse_ports(priv, np);
+			if (ret < 0)
+				goto fail;
+
+			/* initialize the default audio configuration */
+			if (priv->audio.ports[0]) {
+				priv->params.audio_cfg = priv->audio.ports[0];
+				priv->params.audio_format =
+						priv->audio.port_types[0];
+				priv->params.audio_clk_cfg =
+					priv->params.audio_format ==
+							AFMT_SPDIF ? 0 : 1;
+			}
+		} else {
+
+			/* optional video properties */
+			ret = of_property_read_u32(np, "video-ports", &video);
+			if (ret == 0) {
+				priv->vip_cntrl_0 = video >> 16;
+				priv->vip_cntrl_1 = video >> 8;
+				priv->vip_cntrl_2 = video;
+			}
+		}
 	}
 
 	return 0;
diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h
new file mode 100644
index 0000000..bef1da7
--- /dev/null
+++ b/include/sound/tda998x.h
@@ -0,0 +1,8 @@ 
+#ifndef SND_TDA998X_H
+#define SND_TDA998X_H
+
+struct tda998x_audio {
+	u8 ports[2];			/* AP value */
+	u8 port_types[2];		/* AFMT_xxx */
+};
+#endif