From: "Ondřej Jirman" <megi@xff.cz> To: "Jernej Škrabec" <jernej.skrabec@gmail.com> Cc: linux-kernel@vger.kernel.org, kernel test robot <lkp@intel.com>, Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, Chen-Yu Tsai <wens@csie.org>, Samuel Holland <samuel@sholland.org>, Rob Herring <robh@kernel.org>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Claudiu Beznea <claudiu.beznea@tuxon.dev>, Arnaud Ferraris <arnaud.ferraris@collabora.com>, "open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." <linux-sound@vger.kernel.org>, "moderated list:ARM/Allwinner sunXi SoC support" <linux-arm-kernel@lists.infradead.org>, "open list:ARM/Allwinner sunXi SoC support" <linux-sunxi@lists.linux.dev> Subject: Re: [PATCH] ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled Date: Thu, 28 Mar 2024 09:52:17 +0100 [thread overview] Message-ID: <mr4oqcufxojw2jocj3hbjz6m6sthoauby5ggbps7ahy7hgxh75@2ztouzzmso4n> (raw) In-Reply-To: <1785985.VLH7GnMWUR@jernej-laptop> 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; > > } > > > > > >
WARNING: multiple messages have this Message-ID (diff)
From: "Ondřej Jirman" <megi@xff.cz> To: "Jernej Škrabec" <jernej.skrabec@gmail.com> Cc: linux-kernel@vger.kernel.org, kernel test robot <lkp@intel.com>, Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, Chen-Yu Tsai <wens@csie.org>, Samuel Holland <samuel@sholland.org>, Rob Herring <robh@kernel.org>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Claudiu Beznea <claudiu.beznea@tuxon.dev>, Arnaud Ferraris <arnaud.ferraris@collabora.com>, "open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." <linux-sound@vger.kernel.org>, "moderated list:ARM/Allwinner sunXi SoC support" <linux-arm-kernel@lists.infradead.org>, "open list:ARM/Allwinner sunXi SoC support" <linux-sunxi@lists.linux.dev> Subject: Re: [PATCH] ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled Date: Thu, 28 Mar 2024 09:52:17 +0100 [thread overview] Message-ID: <mr4oqcufxojw2jocj3hbjz6m6sthoauby5ggbps7ahy7hgxh75@2ztouzzmso4n> (raw) In-Reply-To: <1785985.VLH7GnMWUR@jernej-laptop> 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; > > } > > > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-28 8:52 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-27 8:05 [PATCH] ASoC: sun8i-codec: Fix build with CONFIG_SND_JACK_INPUT_DEV disabled Ondřej Jirman 2024-03-27 8:05 ` Ondřej Jirman 2024-03-27 19:43 ` Jernej Škrabec 2024-03-27 19:43 ` Jernej Škrabec 2024-03-28 8:52 ` Ondřej Jirman [this message] 2024-03-28 8:52 ` Ondřej Jirman 2024-03-28 14:10 ` Mark Brown 2024-03-28 14:10 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=mr4oqcufxojw2jocj3hbjz6m6sthoauby5ggbps7ahy7hgxh75@2ztouzzmso4n \ --to=megi@xff.cz \ --cc=angelogioacchino.delregno@collabora.com \ --cc=arnaud.ferraris@collabora.com \ --cc=broonie@kernel.org \ --cc=claudiu.beznea@tuxon.dev \ --cc=jernej.skrabec@gmail.com \ --cc=lgirdwood@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sound@vger.kernel.org \ --cc=linux-sunxi@lists.linux.dev \ --cc=lkp@intel.com \ --cc=perex@perex.cz \ --cc=robh@kernel.org \ --cc=samuel@sholland.org \ --cc=tiwai@suse.com \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.