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

2019-06-13 Thread Russell King - ARM Linux admin
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  
> 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 
> > ---
> 
> 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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-06-13 Thread Sven Van Asbroeck
On Tue, Jun 11, 2019 at 7:01 AM Russell King  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 
> ---

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) {
   ^
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-06-12 Thread Russell King
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 
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 56 ++-
 1 file changed, 31 insertions(+), 25 deletions(-)

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, >cea);
+   return tda998x_write_aif(priv, >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,