diff mbox series

[01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

Message ID E1haeX3-0001xe-1A@rmk-PC.armlinux.org.uk (mailing list archive)
State New, archived
Headers show
Series tda998x updates | expand

Commit Message

Russell King (Oracle) June 11, 2019, 11:01 a.m. UTC
Introduce a structure to hold the register values to be programmed while
programming the TDA998x audio settings.  This is currently a stub
structure, which will be populated in subsequent commits.

When we initialise this from the platform data, only do so if there is a
valid audio format specification.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 56 ++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 25 deletions(-)

Comments

Sven Van Asbroeck June 12, 2019, 3:24 p.m. UTC | #1
On Tue, Jun 11, 2019 at 7:01 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Introduce a structure to hold the register values to be programmed while
> programming the TDA998x audio settings.  This is currently a stub
> structure, which will be populated in subsequent commits.
>
> When we initialise this from the platform data, only do so if there is a
> valid audio format specification.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

Nit:
Fix compile errors? Note that these errors disappear if patch 2/13
is applied, but maybe we want to make sure that every patch compiles
so git bisect does not break?

drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_audio_hw_params’:
drivers/gpu/drm/i2c/tda998x_drv.c:1011:10: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
     audio.config = priv->audio_port[i].config;
          ^
drivers/gpu/drm/i2c/tda998x_drv.c:1012:8: error: ‘struct
tda998x_audio_settings’ has no member named ‘format’
   audio.format = AFMT_I2S;
        ^
drivers/gpu/drm/i2c/tda998x_drv.c:1017:10: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
     audio.config = priv->audio_port[i].config;
          ^
drivers/gpu/drm/i2c/tda998x_drv.c:1018:8: error: ‘struct
tda998x_audio_settings’ has no member named ‘format’
   audio.format = AFMT_SPDIF;
        ^
drivers/gpu/drm/i2c/tda998x_drv.c:1025:11: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
  if (audio.config == 0) {
           ^
Russell King (Oracle) June 13, 2019, 9:52 a.m. UTC | #2
On Wed, Jun 12, 2019 at 11:24:46AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:01 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Introduce a structure to hold the register values to be programmed while
> > programming the TDA998x audio settings.  This is currently a stub
> > structure, which will be populated in subsequent commits.
> >
> > When we initialise this from the platform data, only do so if there is a
> > valid audio format specification.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> Nit:
> Fix compile errors? Note that these errors disappear if patch 2/13
> is applied, but maybe we want to make sure that every patch compiles
> so git bisect does not break?
> 
> drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_audio_hw_params’:
> drivers/gpu/drm/i2c/tda998x_drv.c:1011:10: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>      audio.config = priv->audio_port[i].config;
>           ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1012:8: error: ‘struct
> tda998x_audio_settings’ has no member named ‘format’
>    audio.format = AFMT_I2S;
>         ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1017:10: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>      audio.config = priv->audio_port[i].config;
>           ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1018:8: error: ‘struct
> tda998x_audio_settings’ has no member named ‘format’
>    audio.format = AFMT_SPDIF;
>         ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1025:11: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>   if (audio.config == 0) {
>            ^

Thanks for reporting.  All fixed.

However, I'm concerned that the 0-day builder never found these despite
this code being published for many weeks.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb515..e478633d9a7a 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -40,6 +40,10 @@  struct tda998x_audio_port {
 	u8 config;		/* AP value */
 };
 
+struct tda998x_audio_settings {
+	struct tda998x_audio_params params;
+};
+
 struct tda998x_priv {
 	struct i2c_client *cec;
 	struct i2c_client *hdmi;
@@ -54,7 +58,7 @@  struct tda998x_priv {
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
 	unsigned long tmds_clock;
-	struct tda998x_audio_params audio_params;
+	struct tda998x_audio_settings audio;
 
 	struct platform_device *audio_pdev;
 	struct mutex audio_mutex;
@@ -869,18 +873,17 @@  static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
 	}
 }
 
-static int
-tda998x_configure_audio(struct tda998x_priv *priv,
-			struct tda998x_audio_params *params)
+static int tda998x_configure_audio(struct tda998x_priv *priv,
+				 const struct tda998x_audio_settings *settings)
 {
 	u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
 	u32 n;
 
 	/* Enable audio ports */
-	reg_write(priv, REG_ENA_AP, params->config);
+	reg_write(priv, REG_ENA_AP, settings->params.config);
 
 	/* Set audio input source */
-	switch (params->format) {
+	switch (settings->params.format) {
 	case AFMT_SPDIF:
 		reg_write(priv, REG_ENA_ACLK, 0);
 		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF);
@@ -894,7 +897,7 @@  tda998x_configure_audio(struct tda998x_priv *priv,
 		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S);
 		clksel_aip = AIP_CLKSEL_AIP_I2S;
 		clksel_fs = AIP_CLKSEL_FS_ACLK;
-		switch (params->sample_width) {
+		switch (settings->params.sample_width) {
 		case 16:
 			cts_n = CTS_N_M(3) | CTS_N_K(1);
 			break;
@@ -932,7 +935,7 @@  tda998x_configure_audio(struct tda998x_priv *priv,
 		adiv++;			/* AUDIO_DIV_SERCLK_16 */
 
 	/* S/PDIF asks for a larger divider */
-	if (params->format == AFMT_SPDIF)
+	if (settings->params.format == AFMT_SPDIF)
 		adiv++;			/* AUDIO_DIV_SERCLK_16 or _32 */
 
 	reg_write(priv, REG_AUDIO_DIV, adiv);
@@ -941,7 +944,7 @@  tda998x_configure_audio(struct tda998x_priv *priv,
 	 * This is the approximate value of N, which happens to be
 	 * the recommended values for non-coherent clocks.
 	 */
-	n = 128 * params->sample_rate / 1000;
+	n = 128 * settings->params.sample_rate / 1000;
 
 	/* Write the CTS and N values */
 	buf[0] = 0x44;
@@ -963,17 +966,17 @@  tda998x_configure_audio(struct tda998x_priv *priv,
 	 * The REG_CH_STAT_B-registers skip IEC958 AES2 byte, because
 	 * there is a separate register for each I2S wire.
 	 */
-	buf[0] = params->status[0];
-	buf[1] = params->status[1];
-	buf[2] = params->status[3];
-	buf[3] = params->status[4];
+	buf[0] = settings->params.status[0];
+	buf[1] = settings->params.status[1];
+	buf[2] = settings->params.status[3];
+	buf[3] = settings->params.status[4];
 	reg_write_range(priv, REG_CH_STAT_B(0), buf, 4);
 
 	tda998x_audio_mute(priv, true);
 	msleep(20);
 	tda998x_audio_mute(priv, false);
 
-	return tda998x_write_aif(priv, &params->cea);
+	return tda998x_write_aif(priv, &settings->params.cea);
 }
 
 static int tda998x_audio_hw_params(struct device *dev, void *data,
@@ -982,14 +985,16 @@  static int tda998x_audio_hw_params(struct device *dev, void *data,
 {
 	struct tda998x_priv *priv = dev_get_drvdata(dev);
 	int i, ret;
-	struct tda998x_audio_params audio = {
-		.sample_width = params->sample_width,
-		.sample_rate = params->sample_rate,
-		.cea = params->cea,
+	struct tda998x_audio_settings audio = {
+		.params = {
+			.sample_width = params->sample_width,
+			.sample_rate = params->sample_rate,
+			.cea = params->cea,
+		},
 	};
 
-	memcpy(audio.status, params->iec.status,
-	       min(sizeof(audio.status), sizeof(params->iec.status)));
+	memcpy(audio.params.status, params->iec.status,
+	       min(sizeof(audio.params.status), sizeof(params->iec.status)));
 
 	switch (daifmt->fmt) {
 	case HDMI_I2S:
@@ -1029,7 +1034,7 @@  static int tda998x_audio_hw_params(struct device *dev, void *data,
 		ret = 0;
 
 	if (ret == 0)
-		priv->audio_params = audio;
+		priv->audio = audio;
 	mutex_unlock(&priv->audio_mutex);
 
 	return ret;
@@ -1043,7 +1048,7 @@  static void tda998x_audio_shutdown(struct device *dev, void *data)
 
 	reg_write(priv, REG_ENA_AP, 0);
 
-	priv->audio_params.format = AFMT_UNUSED;
+	priv->audio.params.format = AFMT_UNUSED;
 
 	mutex_unlock(&priv->audio_mutex);
 }
@@ -1549,9 +1554,9 @@  static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
-		if (priv->audio_params.format != AFMT_UNUSED &&
+		if (priv->audio.params.format != AFMT_UNUSED &&
 		    priv->sink_has_audio)
-			tda998x_configure_audio(priv, &priv->audio_params);
+			tda998x_configure_audio(priv, &priv->audio);
 	}
 
 	mutex_unlock(&priv->audio_mutex);
@@ -1626,7 +1631,8 @@  static void tda998x_set_config(struct tda998x_priv *priv,
 			    VIP_CNTRL_2_SWAP_F(p->swap_f) |
 			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
 
-	priv->audio_params = p->audio_params;
+	if (p->audio_params.format != AFMT_UNUSED)
+		priv->audio.params = p->audio_params;
 }
 
 static void tda998x_destroy(struct device *dev)