NACK/Cmnt: [SRU][R][PATCH v5 3/8] ASoC: SDCA: Remove devm from primary IRQ cleanup
Andrei Gherzan
andrei.gherzan at canonical.com
Fri Sep 11 12:02:03 UTC 2026
On 26/09/11 04:44PM, 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>
> (cherry picked from commit 0880082c27b6251cc3fea307dffa6899ad163e8b)
This is a backported patch.
> Signed-off-by: Chris Chiu <chris.chiu at canonical.com>
>
> [ChrisChiu: Dropped dead type_get_mask(); moved sdca_jack_free_state before
> sdca_jack_set_jack; added free_priv handlers and cleanup]
Some unexpected formatting structure here: SOB before the backport reason and a
spurious new line.
> ---
> v3 -> v4 change:
> - factor duplicate reporting into type_get_mask and modify related path
>
> v4 -> v5 change:
> - drop the dead helper type_get_mask and move the sdca_jack functions
> accordingly
>
> 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 | 20 ++++++++++++++++++--
> 6 files changed, 43 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)
>
> 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..6a6f03924a36 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,23 @@ 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)
> +{
> + struct jack_state *jack_state = interrupt->priv;
> +
> + if (!jack_state)
> + return;
> +
> + /* devm allocation: just clear the pointer */
Why did we deviate from upstream here? Shouldn't we have a kfree as well? What
is even more confusing is that patch 8 later silently brings back kfree.
> + interrupt->priv = NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(sdca_jack_free_state, "SND_SOC_SDCA");
> +
> +
> /**
> * sdca_jack_set_jack - attach an ASoC jack to SDCA
> * @info: SDCA interrupt information.
--
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260911/effc8549/attachment.sig>
More information about the kernel-team
mailing list