Skip to content

Commit 4e53116

Browse files
charleskeepaxbroonie
authored andcommitted
ASoC: SDCA: Fix errors in IRQ cleanup
IRQs are enabled through sdca_irq_populate() from component probe using devm_request_threaded_irq(), this however means the IRQs can persist if the sound card is torn down. Some of the IRQ handlers store references to the card and the kcontrols which can then fail. Some detail of the crash was explained in [1]. Generally it is not advised to use devm outside of bus probe, so the code is updated to not use devm. The IRQ requests are not moved to bus probe time as it makes passing the snd_soc_component into the IRQs very awkward and would the require a second step once the component is available, so it is simpler to just register the IRQs at this point, even though that necessitates some manual cleanup. Link: https://lore.kernel.org/linux-sound/20260310183829.2907805-1-gaggery.tsai@intel.com/ [1] Fixes: b126394 ("ASoC: SDCA: Generic interrupt support") Reported-by: Gaggery Tsai <gaggery.tsai@intel.com> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://patch.msgid.link/20260316141449.2950215-1-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
1 parent 80a7916 commit 4e53116

3 files changed

Lines changed: 87 additions & 4 deletions

File tree

include/sound/sdca_interrupts.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ struct sdca_interrupt_info {
6969
int sdca_irq_request(struct device *dev, struct sdca_interrupt_info *interrupt_info,
7070
int sdca_irq, const char *name, irq_handler_t handler,
7171
void *data);
72+
void sdca_irq_free(struct device *dev, struct sdca_interrupt_info *interrupt_info,
73+
int sdca_irq, const char *name, void *data);
7274
int sdca_irq_data_populate(struct device *dev, struct regmap *function_regmap,
7375
struct snd_soc_component *component,
7476
struct sdca_function_data *function,
@@ -81,6 +83,9 @@ int sdca_irq_populate_early(struct device *dev, struct regmap *function_regmap,
8183
int sdca_irq_populate(struct sdca_function_data *function,
8284
struct snd_soc_component *component,
8385
struct sdca_interrupt_info *info);
86+
void sdca_irq_cleanup(struct sdca_function_data *function,
87+
struct snd_soc_component *component,
88+
struct sdca_interrupt_info *info);
8489
struct sdca_interrupt_info *sdca_irq_allocate(struct device *dev,
8590
struct regmap *regmap, int irq);
8691

sound/soc/sdca/sdca_class_function.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,14 @@ static int class_function_component_probe(struct snd_soc_component *component)
198198
return sdca_irq_populate(drv->function, component, core->irq_info);
199199
}
200200

201+
static void class_function_component_remove(struct snd_soc_component *component)
202+
{
203+
struct class_function_drv *drv = snd_soc_component_get_drvdata(component);
204+
struct sdca_class_drv *core = drv->core;
205+
206+
sdca_irq_cleanup(drv->function, component, core->irq_info);
207+
}
208+
201209
static int class_function_set_jack(struct snd_soc_component *component,
202210
struct snd_soc_jack *jack, void *d)
203211
{
@@ -209,6 +217,7 @@ static int class_function_set_jack(struct snd_soc_component *component,
209217

210218
static const struct snd_soc_component_driver class_function_component_drv = {
211219
.probe = class_function_component_probe,
220+
.remove = class_function_component_remove,
212221
.endianness = 1,
213222
};
214223

sound/soc/sdca/sdca_interrupts.c

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,7 @@ static int sdca_irq_request_locked(struct device *dev,
252252
if (irq < 0)
253253
return irq;
254254

255-
ret = devm_request_threaded_irq(dev, irq, NULL, handler,
256-
IRQF_ONESHOT, name, data);
255+
ret = request_threaded_irq(irq, NULL, handler, IRQF_ONESHOT, name, data);
257256
if (ret)
258257
return ret;
259258

@@ -264,6 +263,22 @@ static int sdca_irq_request_locked(struct device *dev,
264263
return 0;
265264
}
266265

266+
static void sdca_irq_free_locked(struct device *dev, struct sdca_interrupt_info *info,
267+
int sdca_irq, const char *name, void *data)
268+
{
269+
int irq;
270+
271+
irq = regmap_irq_get_virq(info->irq_data, sdca_irq);
272+
if (irq < 0)
273+
return;
274+
275+
free_irq(irq, data);
276+
277+
info->irqs[sdca_irq].irq = 0;
278+
279+
dev_dbg(dev, "freed irq %d for %s\n", irq, name);
280+
}
281+
267282
/**
268283
* sdca_irq_request - request an individual SDCA interrupt
269284
* @dev: Pointer to the struct device against which things should be allocated.
@@ -302,6 +317,30 @@ int sdca_irq_request(struct device *dev, struct sdca_interrupt_info *info,
302317
}
303318
EXPORT_SYMBOL_NS_GPL(sdca_irq_request, "SND_SOC_SDCA");
304319

320+
/**
321+
* sdca_irq_free - free an individual SDCA interrupt
322+
* @dev: Pointer to the struct device.
323+
* @info: Pointer to the interrupt information structure.
324+
* @sdca_irq: SDCA interrupt position.
325+
* @name: Name to be given to the IRQ.
326+
* @data: Private data pointer that will be passed to the handler.
327+
*
328+
* Typically this is handled internally by sdca_irq_cleanup, however if
329+
* a device requires custom IRQ handling this can be called manually before
330+
* calling sdca_irq_cleanup, which will then skip that IRQ whilst processing.
331+
*/
332+
void sdca_irq_free(struct device *dev, struct sdca_interrupt_info *info,
333+
int sdca_irq, const char *name, void *data)
334+
{
335+
if (sdca_irq < 0 || sdca_irq >= SDCA_MAX_INTERRUPTS)
336+
return;
337+
338+
guard(mutex)(&info->irq_lock);
339+
340+
sdca_irq_free_locked(dev, info, sdca_irq, name, data);
341+
}
342+
EXPORT_SYMBOL_NS_GPL(sdca_irq_free, "SND_SOC_SDCA");
343+
305344
/**
306345
* sdca_irq_data_populate - Populate common interrupt data
307346
* @dev: Pointer to the Function device.
@@ -328,8 +367,8 @@ int sdca_irq_data_populate(struct device *dev, struct regmap *regmap,
328367
if (!dev)
329368
return -ENODEV;
330369

331-
name = devm_kasprintf(dev, GFP_KERNEL, "%s %s %s", function->desc->name,
332-
entity->label, control->label);
370+
name = kasprintf(GFP_KERNEL, "%s %s %s", function->desc->name,
371+
entity->label, control->label);
333372
if (!name)
334373
return -ENOMEM;
335374

@@ -516,6 +555,36 @@ int sdca_irq_populate(struct sdca_function_data *function,
516555
}
517556
EXPORT_SYMBOL_NS_GPL(sdca_irq_populate, "SND_SOC_SDCA");
518557

558+
/**
559+
* sdca_irq_cleanup - Free all the individual IRQs for an SDCA Function
560+
* @function: Pointer to the SDCA Function.
561+
* @component: Pointer to the ASoC component for the Function.
562+
* @info: Pointer to the SDCA interrupt info for this device.
563+
*
564+
* Typically this would be called from the driver for a single SDCA Function.
565+
*/
566+
void sdca_irq_cleanup(struct sdca_function_data *function,
567+
struct snd_soc_component *component,
568+
struct sdca_interrupt_info *info)
569+
{
570+
struct device *dev = component->dev;
571+
int i;
572+
573+
guard(mutex)(&info->irq_lock);
574+
575+
for (i = 0; i < SDCA_MAX_INTERRUPTS; i++) {
576+
struct sdca_interrupt *interrupt = &info->irqs[i];
577+
578+
if (interrupt->function != function || !interrupt->irq)
579+
continue;
580+
581+
sdca_irq_free_locked(dev, info, i, interrupt->name, interrupt);
582+
583+
kfree(interrupt->name);
584+
}
585+
}
586+
EXPORT_SYMBOL_NS_GPL(sdca_irq_cleanup, "SND_SOC_SDCA");
587+
519588
/**
520589
* sdca_irq_allocate - allocate an SDCA interrupt structure for a device
521590
* @sdev: Device pointer against which things should be allocated.

0 commit comments

Comments
 (0)