From: Ondrej Jirman <[email protected]>
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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
---
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;
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;
}
--
2.44.0
Dne sreda, 27. marec 2024 ob 09:05:34 CET je Ondřej Jirman napisal(a):
> From: Ondrej Jirman <[email protected]>
>
> 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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> ---
> 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;
> }
>
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 <[email protected]>
> >
> > 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 <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > ---
> > 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/[email protected]/
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;
> > }
> >
>
>
>
>
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