diff mbox series

ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled

Message ID 20240327080542.3649840-1-megi@xff.cz (mailing list archive)
State Accepted
Commit 559aebe45a054a479fdbd2a3dfba999ffd73cc9d
Headers show
Series ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled | expand

Commit Message

Ondřej Jirman March 27, 2024, 8:05 a.m. UTC 
From: Ondrej Jirman <megi@xff.cz>

When CONFIG_SND_JACK_INPUT_DEV is disabled, struct snd_jack doesn't
have 'type' field. We can't rely on this field being always present,
so store the jack_type in the codec driver itself.

Signed-off-by: Ondrej Jirman <megi@xff.cz>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202403261243.kcXCPAYs-lkp@intel.com/
---
This is a followup for https://lore.kernel.org/lkml/2vhd45kylttgonosdcfn7ugwyy2vx56gijwf3ealz4tel65iqr@cs3kxrl6d2hh/

 sound/soc/sunxi/sun8i-codec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Jernej Škrabec March 27, 2024, 7:43 p.m. UTC | #1 
Dne sreda, 27. marec 2024 ob 09:05:34 CET je Ondřej Jirman napisal(a):
> From: Ondrej Jirman <megi@xff.cz>
> 
> When CONFIG_SND_JACK_INPUT_DEV is disabled, struct snd_jack doesn't
> have 'type' field. We can't rely on this field being always present,
> so store the jack_type in the codec driver itself.
> 
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202403261243.kcXCPAYs-lkp@intel.com/
> ---
> This is a followup for https://lore.kernel.org/lkml/2vhd45kylttgonosdcfn7ugwyy2vx56gijwf3ealz4tel65iqr@cs3kxrl6d2hh/
> 
>  sound/soc/sunxi/sun8i-codec.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
> index 43795362fed0..b5dafb749c3f 100644
> --- a/sound/soc/sunxi/sun8i-codec.c
> +++ b/sound/soc/sunxi/sun8i-codec.c
> @@ -232,6 +232,7 @@ struct sun8i_codec {
>  	struct delayed_work		jack_work;
>  	int				jack_irq;
>  	int				jack_status;
> +	int				jack_type;

Where is this variable set? Below I see just usage of its value.

Best regards,
Jernej

>  	int				jack_last_sample;
>  	ktime_t				jack_hbias_ready;
>  	struct mutex			jack_mutex;
> @@ -1352,7 +1353,6 @@ static void sun8i_codec_jack_work(struct work_struct *work)
>  	struct sun8i_codec *scodec = container_of(work, struct sun8i_codec,
>  						  jack_work.work);
>  	unsigned int mdata;
> -	int type_mask = scodec->jack->jack->type;
>  	int type;
>  
>  	guard(mutex)(&scodec->jack_mutex);
> @@ -1363,7 +1363,7 @@ static void sun8i_codec_jack_work(struct work_struct *work)
>  
>  		scodec->jack_last_sample = -1;
>  
> -		if (type_mask & SND_JACK_MICROPHONE) {
> +		if (scodec->jack_type & SND_JACK_MICROPHONE) {
>  			/*
>  			 * If we were in disconnected state, we enable HBIAS and
>  			 * wait 600ms before reading initial HDATA value.
> @@ -1376,7 +1376,7 @@ static void sun8i_codec_jack_work(struct work_struct *work)
>  			scodec->jack_status = SUN8I_JACK_STATUS_WAITING_HBIAS;
>  		} else {
>  			snd_soc_jack_report(scodec->jack, SND_JACK_HEADPHONE,
> -					    type_mask);
> +					    scodec->jack_type);
>  			scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
>  		}
>  	} else if (scodec->jack_status == SUN8I_JACK_STATUS_WAITING_HBIAS) {
> @@ -1417,17 +1417,17 @@ static void sun8i_codec_jack_work(struct work_struct *work)
>  		if (type == SND_JACK_HEADPHONE)
>  			sun8i_codec_set_hmic_bias(scodec, false);
>  
> -		snd_soc_jack_report(scodec->jack, type, type_mask);
> +		snd_soc_jack_report(scodec->jack, type, scodec->jack_type);
>  		scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
>  	} else if (scodec->jack_status == SUN8I_JACK_STATUS_CONNECTED) {
>  		if (scodec->last_hmic_irq != SUN8I_HMIC_STS_JACK_OUT_IRQ_ST)
>  			return;
>  
>  		scodec->jack_status = SUN8I_JACK_STATUS_DISCONNECTED;
> -		if (type_mask & SND_JACK_MICROPHONE)
> +		if (scodec->jack_type & SND_JACK_MICROPHONE)
>  			sun8i_codec_set_hmic_bias(scodec, false);
>  
> -		snd_soc_jack_report(scodec->jack, 0, type_mask);
> +		snd_soc_jack_report(scodec->jack, 0, scodec->jack_type);
>  	}
>  }
>  
> @@ -1491,7 +1491,7 @@ static irqreturn_t sun8i_codec_jack_irq(int irq, void *dev_id)
>  		if (scodec->jack_last_sample >= 0 &&
>  		    scodec->jack_last_sample == value)
>  			snd_soc_jack_report(scodec->jack, type,
> -					    scodec->jack->jack->type);
> +					    scodec->jack_type);
>  
>  		scodec->jack_last_sample = value;
>  	}
>
Ondřej Jirman March 28, 2024, 8:52 a.m. UTC | #2 
Hello Jernej,

On Wed, Mar 27, 2024 at 08:43:31PM +0100, Jernej Škrabec wrote:
> Dne sreda, 27. marec 2024 ob 09:05:34 CET je Ondřej Jirman napisal(a):
> > From: Ondrej Jirman <megi@xff.cz>
> > 
> > When CONFIG_SND_JACK_INPUT_DEV is disabled, struct snd_jack doesn't
> > have 'type' field. We can't rely on this field being always present,
> > so store the jack_type in the codec driver itself.
> > 
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202403261243.kcXCPAYs-lkp@intel.com/
> > ---
> > This is a followup for https://lore.kernel.org/lkml/2vhd45kylttgonosdcfn7ugwyy2vx56gijwf3ealz4tel65iqr@cs3kxrl6d2hh/
> > 
> >  sound/soc/sunxi/sun8i-codec.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
> > index 43795362fed0..b5dafb749c3f 100644
> > --- a/sound/soc/sunxi/sun8i-codec.c
> > +++ b/sound/soc/sunxi/sun8i-codec.c
> > @@ -232,6 +232,7 @@ struct sun8i_codec {
> >  	struct delayed_work		jack_work;
> >  	int				jack_irq;
> >  	int				jack_status;
> > +	int				jack_type;
> 
> Where is this variable set? Below I see just usage of its value.

It's going to be set in probe function, once generic DT bindings for jack-type
are figured out. This is mentioned in the v3 cover letter for
https://lore.kernel.org/lkml/20240302140042.1990256-1-megi@xff.cz/

I plan on sending support for generic property mentioned here
https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-component.c#L288
but it may need several iterations, and fail to get merged for various reasons
and I may need to fall back to vendor DT property, then.

If you want to test/use the jack detection on some A64 board now, what's needed
to enable this is to add:

  scodec->jack_type = SND_JACK_HEADSET | SUN8I_CODEC_BUTTONS;

  (or just SND_JACK_HEADPHONE, depednding on jack type)

to probe function and add get_jack_type callback to sun8i_soc_component:

  static int sun8i_codec_component_get_jack_type(struct snd_soc_component *component)
  {
	struct sun8i_codec *scodec = snd_soc_component_get_drvdata(component);

	return scodec->jack_type;
  }

Then detection will get enabled on A64 SoC based boards that use simple-audio-card.
(if codec is in simple-audio-card,aux-devs)

Simple card iterates through aux devices and adds jacks for the ones that return
something from component's get_jack_type() callback:

   https://elixir.bootlin.com/linux/latest/source/sound/soc/generic/simple-card-utils.c#L822

kind regards,
	o.

> Best regards,
> Jernej
> 
> >  	int				jack_last_sample;
> >  	ktime_t				jack_hbias_ready;
> >  	struct mutex			jack_mutex;
> > @@ -1352,7 +1353,6 @@ static void sun8i_codec_jack_work(struct work_struct *work)
> >  	struct sun8i_codec *scodec = container_of(work, struct sun8i_codec,
> >  						  jack_work.work);
> >  	unsigned int mdata;
> > -	int type_mask = scodec->jack->jack->type;
> >  	int type;
> >  
> >  	guard(mutex)(&scodec->jack_mutex);
> > @@ -1363,7 +1363,7 @@ static void sun8i_codec_jack_work(struct work_struct *work)
> >  
> >  		scodec->jack_last_sample = -1;
> >  
> > -		if (type_mask & SND_JACK_MICROPHONE) {
> > +		if (scodec->jack_type & SND_JACK_MICROPHONE) {
> >  			/*
> >  			 * If we were in disconnected state, we enable HBIAS and
> >  			 * wait 600ms before reading initial HDATA value.
> > @@ -1376,7 +1376,7 @@ static void sun8i_codec_jack_work(struct work_struct *work)
> >  			scodec->jack_status = SUN8I_JACK_STATUS_WAITING_HBIAS;
> >  		} else {
> >  			snd_soc_jack_report(scodec->jack, SND_JACK_HEADPHONE,
> > -					    type_mask);
> > +					    scodec->jack_type);
> >  			scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
> >  		}
> >  	} else if (scodec->jack_status == SUN8I_JACK_STATUS_WAITING_HBIAS) {
> > @@ -1417,17 +1417,17 @@ static void sun8i_codec_jack_work(struct work_struct *work)
> >  		if (type == SND_JACK_HEADPHONE)
> >  			sun8i_codec_set_hmic_bias(scodec, false);
> >  
> > -		snd_soc_jack_report(scodec->jack, type, type_mask);
> > +		snd_soc_jack_report(scodec->jack, type, scodec->jack_type);
> >  		scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
> >  	} else if (scodec->jack_status == SUN8I_JACK_STATUS_CONNECTED) {
> >  		if (scodec->last_hmic_irq != SUN8I_HMIC_STS_JACK_OUT_IRQ_ST)
> >  			return;
> >  
> >  		scodec->jack_status = SUN8I_JACK_STATUS_DISCONNECTED;
> > -		if (type_mask & SND_JACK_MICROPHONE)
> > +		if (scodec->jack_type & SND_JACK_MICROPHONE)
> >  			sun8i_codec_set_hmic_bias(scodec, false);
> >  
> > -		snd_soc_jack_report(scodec->jack, 0, type_mask);
> > +		snd_soc_jack_report(scodec->jack, 0, scodec->jack_type);
> >  	}
> >  }
> >  
> > @@ -1491,7 +1491,7 @@ static irqreturn_t sun8i_codec_jack_irq(int irq, void *dev_id)
> >  		if (scodec->jack_last_sample >= 0 &&
> >  		    scodec->jack_last_sample == value)
> >  			snd_soc_jack_report(scodec->jack, type,
> > -					    scodec->jack->jack->type);
> > +					    scodec->jack_type);
> >  
> >  		scodec->jack_last_sample = value;
> >  	}
> > 
> 
> 
> 
>
Mark Brown March 28, 2024, 2:10 p.m. UTC | #3 
On Wed, 27 Mar 2024 09:05:34 +0100, Ondřej Jirman wrote:
> When CONFIG_SND_JACK_INPUT_DEV is disabled, struct snd_jack doesn't
> have 'type' field. We can't rely on this field being always present,
> so store the jack_type in the codec driver itself.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled
      commit: 559aebe45a054a479fdbd2a3dfba999ffd73cc9d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index 43795362fed0..b5dafb749c3f 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -232,6 +232,7 @@  struct sun8i_codec {
 	struct delayed_work		jack_work;
 	int				jack_irq;
 	int				jack_status;
+	int				jack_type;
 	int				jack_last_sample;
 	ktime_t				jack_hbias_ready;
 	struct mutex			jack_mutex;
@@ -1352,7 +1353,6 @@  static void sun8i_codec_jack_work(struct work_struct *work)
 	struct sun8i_codec *scodec = container_of(work, struct sun8i_codec,
 						  jack_work.work);
 	unsigned int mdata;
-	int type_mask = scodec->jack->jack->type;
 	int type;
 
 	guard(mutex)(&scodec->jack_mutex);
@@ -1363,7 +1363,7 @@  static void sun8i_codec_jack_work(struct work_struct *work)
 
 		scodec->jack_last_sample = -1;
 
-		if (type_mask & SND_JACK_MICROPHONE) {
+		if (scodec->jack_type & SND_JACK_MICROPHONE) {
 			/*
 			 * If we were in disconnected state, we enable HBIAS and
 			 * wait 600ms before reading initial HDATA value.
@@ -1376,7 +1376,7 @@  static void sun8i_codec_jack_work(struct work_struct *work)
 			scodec->jack_status = SUN8I_JACK_STATUS_WAITING_HBIAS;
 		} else {
 			snd_soc_jack_report(scodec->jack, SND_JACK_HEADPHONE,
-					    type_mask);
+					    scodec->jack_type);
 			scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
 		}
 	} else if (scodec->jack_status == SUN8I_JACK_STATUS_WAITING_HBIAS) {
@@ -1417,17 +1417,17 @@  static void sun8i_codec_jack_work(struct work_struct *work)
 		if (type == SND_JACK_HEADPHONE)
 			sun8i_codec_set_hmic_bias(scodec, false);
 
-		snd_soc_jack_report(scodec->jack, type, type_mask);
+		snd_soc_jack_report(scodec->jack, type, scodec->jack_type);
 		scodec->jack_status = SUN8I_JACK_STATUS_CONNECTED;
 	} else if (scodec->jack_status == SUN8I_JACK_STATUS_CONNECTED) {
 		if (scodec->last_hmic_irq != SUN8I_HMIC_STS_JACK_OUT_IRQ_ST)
 			return;
 
 		scodec->jack_status = SUN8I_JACK_STATUS_DISCONNECTED;
-		if (type_mask & SND_JACK_MICROPHONE)
+		if (scodec->jack_type & SND_JACK_MICROPHONE)
 			sun8i_codec_set_hmic_bias(scodec, false);
 
-		snd_soc_jack_report(scodec->jack, 0, type_mask);
+		snd_soc_jack_report(scodec->jack, 0, scodec->jack_type);
 	}
 }
 
@@ -1491,7 +1491,7 @@  static irqreturn_t sun8i_codec_jack_irq(int irq, void *dev_id)
 		if (scodec->jack_last_sample >= 0 &&
 		    scodec->jack_last_sample == value)
 			snd_soc_jack_report(scodec->jack, type,
-					    scodec->jack->jack->type);
+					    scodec->jack_type);
 
 		scodec->jack_last_sample = value;
 	}