[SRU][R][PATCH v4 3/7] ASoC: SDCA: Remove devm from primary IRQ cleanup
Chris Chiu
chris.chiu at canonical.com
Tue Sep 8 01:55:03 UTC 2026
On Mon, Sep 7, 2026 at 6:27 PM Andrei Gherzan
<andrei.gherzan at canonical.com> wrote:
>
> On 26/09/07 02:47PM, Chris Chiu via kernel-team wrote:
> > From: Charles Keepax <ckeepax at opensource.cirrus.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/2163215
> >
> > To provide greater flexibility on when the IRQs are requested for
> > client drivers don't use devm for the primary IRQ request/cleanup
> > helper functions.
> >
> > Signed-off-by: Charles Keepax <ckeepax at opensource.cirrus.com>
> > Link: https://patch.msgid.link/20260721143636.361814-4-ckeepax@opensource.cirrus.com
> > Signed-off-by: Mark Brown <broonie at kernel.org>
> > (backported from commit 0880082c27b6251cc3fea307dffa6899ad163e8b)
> > [ChrisChiu: Conflict in sdca_jack.c due to missing sdca_jack_free_state()
> > function in our branch. Resolved by adding the new sdca_jack_free_state()
> > function before the type_get_mask() function as the upstream intended.]
> > Signed-off-by: Chris Chiu <chris.chiu at canonical.com>
> > ---
> > v3 -> v4 chage:
> > - factor duplicate reporting into type_get_mask and modify related path
> >
> > include/sound/sdca_fdl.h | 2 ++
> > include/sound/sdca_interrupts.h | 2 ++
> > include/sound/sdca_jack.h | 2 ++
> > sound/soc/sdca/sdca_fdl.c | 13 +++++++++--
> > sound/soc/sdca/sdca_interrupts.c | 8 +++++++
> > sound/soc/sdca/sdca_jack.c | 39 ++++++++++++++++++++++++++++++--
> > 6 files changed, 62 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sound/sdca_fdl.h b/include/sound/sdca_fdl.h
> > index fbaf4b384c8a..dc33927b82bd 100644
> > --- a/include/sound/sdca_fdl.h
> > +++ b/include/sound/sdca_fdl.h
> > @@ -67,6 +67,8 @@ struct fdl_state {
> > #if IS_ENABLED(CONFIG_SND_SOC_SDCA_FDL)
>
> I think there is an issue here when CONFIG_SND_SOC_SDCA_FDL is disabled. I see
> that we provide stubs for the existing functions (sdca_fdl_alloc_state,
> sdca_fdl_process, sdca_fdl_sync, sdca_reset_function) but for the newly added
> declaration (sdca_fdl_free_state), there is none. This means that the code
> would fail to build when SND_SOC_SDCA_IRQ=y and SND_SOC_SDCA_FDL=n. Checking a
> bit upstream, it looks like a known and fixed gap: "ASoC: SDCA: Add missing
> stub for sdca_fdl_free_state()".
>
Thanks — good catch. The new sdca_fdl_free_state() prototype lacks a
disabled-config stub,
so builds fail when SND_SOC_SDCA_FDL=n but SDCA IRQs are enabled.
Upstream fixed this
with f6970d8535a9 'ASoC: SDCA: Add missing stub for sdca_fdl_free_state()'.
Plan: add the same no-op stub under the #else so the header always
provides a definition, then
post an updated patch (v5). Is this approach acceptable, or do you
have better suggestions?
> >
> > int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt);
> > +void sdca_fdl_free_state(struct sdca_interrupt *interrupt);
> > +
> > int sdca_fdl_process(struct sdca_interrupt *interrupt);
> > int sdca_fdl_sync(struct device *dev, struct sdca_function_data *function,
> > struct sdca_interrupt_info *info);
> > diff --git a/include/sound/sdca_interrupts.h b/include/sound/sdca_interrupts.h
> > index 38c6c58c2cc7..8a44c19e917c 100644
> > --- a/include/sound/sdca_interrupts.h
> > +++ b/include/sound/sdca_interrupts.h
> > @@ -31,6 +31,7 @@ struct sdca_function_data;
> > * @entity: Pointer to the Entity that the interrupt is associated with.
> > * @control: Pointer to the Control that the interrupt is associated with.
> > * @priv: Pointer to private data for use by the handler.
> > + * @free_priv: Pointer to a function that can be used to free the priv data.
> > * @irq: IRQ number allocated to this interrupt, also used internally to track
> > * the IRQ being assigned.
> > * @early_request: Flag to indicate this IRQ was requested at bus probe time.
> > @@ -47,6 +48,7 @@ struct sdca_interrupt {
> > struct sdca_control *control;
> >
> > void *priv;
> > + void (*free_priv)(struct sdca_interrupt *interrupt);
> >
> > int irq;
> > bool early_request;
> > diff --git a/include/sound/sdca_jack.h b/include/sound/sdca_jack.h
> > index 3ec22046d3eb..8624c51eea26 100644
> > --- a/include/sound/sdca_jack.h
> > +++ b/include/sound/sdca_jack.h
> > @@ -25,6 +25,8 @@ struct jack_state {
> > };
> >
> > int sdca_jack_alloc_state(struct sdca_interrupt *interrupt);
> > +void sdca_jack_free_state(struct sdca_interrupt *interrupt);
> > +
> > int sdca_jack_process(struct sdca_interrupt *interrupt);
> > int sdca_jack_set_jack(struct sdca_interrupt_info *info, struct snd_soc_jack *jack);
> > int sdca_jack_report(struct sdca_interrupt *interrupt);
> > diff --git a/sound/soc/sdca/sdca_fdl.c b/sound/soc/sdca/sdca_fdl.c
> > index 994821a6df61..82e09d960c12 100644
> > --- a/sound/soc/sdca/sdca_fdl.c
> > +++ b/sound/soc/sdca/sdca_fdl.c
> > @@ -481,10 +481,9 @@ EXPORT_SYMBOL_NS_GPL(sdca_fdl_process, "SND_SOC_SDCA");
> > */
> > int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt)
> > {
> > - struct device *dev = interrupt->dev;
> > struct fdl_state *fdl_state;
> >
> > - fdl_state = devm_kzalloc(dev, sizeof(*fdl_state), GFP_KERNEL);
> > + fdl_state = kzalloc_obj(*fdl_state);
> > if (!fdl_state)
> > return -ENOMEM;
> >
> > @@ -499,3 +498,13 @@ int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt)
> > return 0;
> > }
> > EXPORT_SYMBOL_NS_GPL(sdca_fdl_alloc_state, "SND_SOC_SDCA");
> > +
> > +/**
> > + * sdca_fdl_free_state - free state for an FDL interrupt
> > + * @interrupt: SDCA interrupt structure.
> > + */
> > +void sdca_fdl_free_state(struct sdca_interrupt *interrupt)
> > +{
> > + kfree(interrupt->priv);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(sdca_fdl_free_state, "SND_SOC_SDCA");
> > diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
> > index 6ff0a0a12755..0e721306f3cf 100644
> > --- a/sound/soc/sdca/sdca_interrupts.c
> > +++ b/sound/soc/sdca/sdca_interrupts.c
> > @@ -448,6 +448,7 @@ int sdca_irq_populate_early(struct device *dev, struct regmap *regmap,
> > return ret;
> >
> > interrupt->early_request = true;
> > + interrupt->free_priv = sdca_fdl_free_state;
> >
> > ret = sdca_fdl_alloc_state(interrupt);
> > if (ret)
> > @@ -521,6 +522,8 @@ int sdca_irq_populate(struct sdca_function_data *function,
> > handler = function_status_handler;
> > break;
> > case SDCA_CTL_TYPE_S(GE, DETECTED_MODE):
> > + interrupt->free_priv = sdca_jack_free_state;
> > +
> > ret = sdca_jack_alloc_state(interrupt);
> > if (ret)
> > return ret;
> > @@ -528,6 +531,8 @@ int sdca_irq_populate(struct sdca_function_data *function,
> > handler = detected_mode_handler;
> > break;
> > case SDCA_CTL_TYPE_S(XU, FDL_CURRENTOWNER):
> > + interrupt->free_priv = sdca_fdl_free_state;
> > +
> > ret = sdca_fdl_alloc_state(interrupt);
> > if (ret)
> > return ret;
> > @@ -575,6 +580,9 @@ static void sdca_irq_cleanup_flags(struct device *dev,
> >
> > sdca_irq_free_locked(dev, info, i, interrupt->name, interrupt);
> >
> > + if (interrupt->free_priv)
> > + interrupt->free_priv(interrupt);
> > +
> > kfree(interrupt->name);
> > }
> > }
> > diff --git a/sound/soc/sdca/sdca_jack.c b/sound/soc/sdca/sdca_jack.c
> > index 49d317d3b8c8..f2494c94ca33 100644
> > --- a/sound/soc/sdca/sdca_jack.c
> > +++ b/sound/soc/sdca/sdca_jack.c
> > @@ -132,10 +132,9 @@ EXPORT_SYMBOL_NS_GPL(sdca_jack_process, "SND_SOC_SDCA");
> > */
> > int sdca_jack_alloc_state(struct sdca_interrupt *interrupt)
> > {
> > - struct device *dev = interrupt->dev;
> > struct jack_state *jack_state;
> >
> > - jack_state = devm_kzalloc(dev, sizeof(*jack_state), GFP_KERNEL);
> > + jack_state = kzalloc_obj(*jack_state);
> > if (!jack_state)
> > return -ENOMEM;
> >
> > @@ -145,6 +144,42 @@ int sdca_jack_alloc_state(struct sdca_interrupt *interrupt)
> > }
> > EXPORT_SYMBOL_NS_GPL(sdca_jack_alloc_state, "SND_SOC_SDCA");
> >
> > +/**
> > + * sdca_jack_free_state - free state for a jack interrupt
> > + * @interrupt: SDCA interrupt structure.
> > + */
> > +void sdca_jack_free_state(struct sdca_interrupt *interrupt)
> > +{
> > + kfree(interrupt->priv);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(sdca_jack_free_state, "SND_SOC_SDCA");
> > +
> > +static int type_get_mask(enum sdca_terminal_type type)
> > +{
> > + switch (type) {
> > + case SDCA_TERM_TYPE_LINEIN_STEREO:
> > + case SDCA_TERM_TYPE_LINEIN_FRONT_LR:
> > + case SDCA_TERM_TYPE_LINEIN_CENTER_LFE:
> > + case SDCA_TERM_TYPE_LINEIN_SURROUND_LR:
> > + case SDCA_TERM_TYPE_LINEIN_REAR_LR:
> > + return SND_JACK_LINEIN;
> > + case SDCA_TERM_TYPE_LINEOUT_STEREO:
> > + case SDCA_TERM_TYPE_LINEOUT_FRONT_LR:
> > + case SDCA_TERM_TYPE_LINEOUT_CENTER_LFE:
> > + case SDCA_TERM_TYPE_LINEOUT_SURROUND_LR:
> > + case SDCA_TERM_TYPE_LINEOUT_REAR_LR:
> > + return SND_JACK_LINEOUT;
> > + case SDCA_TERM_TYPE_MIC_JACK:
> > + return SND_JACK_MICROPHONE;
> > + case SDCA_TERM_TYPE_HEADPHONE_JACK:
> > + return SND_JACK_HEADPHONE;
> > + case SDCA_TERM_TYPE_HEADSET_JACK:
> > + return SND_JACK_HEADSET;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > /**
> > * sdca_jack_set_jack - attach an ASoC jack to SDCA
> > * @info: SDCA interrupt information.
> > --
> > 2.43.0
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
> --
> Andrei Gherzan
> gpg: rsa4096/D4D94F67AD0E9640
More information about the kernel-team
mailing list