From 4161b4505f1690358ac0a9ee59845a7887336b21 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 13 Jan 2015 10:53:20 +0100 Subject: ALSA: ak411x: Fix stall in work callback When ak4114 work calls its callback and the callback invokes ak4114_reinit(), it stalls due to flush_delayed_work(). For avoiding this, control the reentrance by introducing a refcount. Also flush_delayed_work() is replaced with cancel_delayed_work_sync(). The exactly same bug is present in ak4113.c and fixed as well. Reported-by: Pavel Hofman Acked-by: Jaroslav Kysela Tested-by: Pavel Hofman Cc: Signed-off-by: Takashi Iwai --- sound/i2c/other/ak4113.c | 17 ++++++++--------- sound/i2c/other/ak4114.c | 18 ++++++++---------- 2 files changed, 16 insertions(+), 19 deletions(-) (limited to 'sound/i2c') diff --git a/sound/i2c/other/ak4113.c b/sound/i2c/other/ak4113.c index 1a3a6fa27158..c6bba99a90b2 100644 --- a/sound/i2c/other/ak4113.c +++ b/sound/i2c/other/ak4113.c @@ -56,8 +56,7 @@ static inline unsigned char reg_read(struct ak4113 *ak4113, unsigned char reg) static void snd_ak4113_free(struct ak4113 *chip) { - chip->init = 1; /* don't schedule new work */ - mb(); + atomic_inc(&chip->wq_processing); /* don't schedule new work */ cancel_delayed_work_sync(&chip->work); kfree(chip); } @@ -89,6 +88,7 @@ int snd_ak4113_create(struct snd_card *card, ak4113_read_t *read, chip->write = write; chip->private_data = private_data; INIT_DELAYED_WORK(&chip->work, ak4113_stats); + atomic_set(&chip->wq_processing, 0); for (reg = 0; reg < AK4113_WRITABLE_REGS ; reg++) chip->regmap[reg] = pgm[reg]; @@ -139,13 +139,11 @@ static void ak4113_init_regs(struct ak4113 *chip) void snd_ak4113_reinit(struct ak4113 *chip) { - chip->init = 1; - mb(); - flush_delayed_work(&chip->work); + if (atomic_inc_return(&chip->wq_processing) == 1) + cancel_delayed_work_sync(&chip->work); ak4113_init_regs(chip); /* bring up statistics / event queing */ - chip->init = 0; - if (chip->kctls[0]) + if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } EXPORT_SYMBOL_GPL(snd_ak4113_reinit); @@ -632,8 +630,9 @@ static void ak4113_stats(struct work_struct *work) { struct ak4113 *chip = container_of(work, struct ak4113, work.work); - if (!chip->init) + if (atomic_inc_return(&chip->wq_processing) == 1) snd_ak4113_check_rate_and_errors(chip, chip->check_flags); - schedule_delayed_work(&chip->work, HZ / 10); + if (atomic_dec_and_test(&chip->wq_processing)) + schedule_delayed_work(&chip->work, HZ / 10); } diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c index c7f56339415d..b70e6eccbd03 100644 --- a/sound/i2c/other/ak4114.c +++ b/sound/i2c/other/ak4114.c @@ -66,8 +66,7 @@ static void reg_dump(struct ak4114 *ak4114) static void snd_ak4114_free(struct ak4114 *chip) { - chip->init = 1; /* don't schedule new work */ - mb(); + atomic_inc(&chip->wq_processing); /* don't schedule new work */ cancel_delayed_work_sync(&chip->work); kfree(chip); } @@ -100,6 +99,7 @@ int snd_ak4114_create(struct snd_card *card, chip->write = write; chip->private_data = private_data; INIT_DELAYED_WORK(&chip->work, ak4114_stats); + atomic_set(&chip->wq_processing, 0); for (reg = 0; reg < 6; reg++) chip->regmap[reg] = pgm[reg]; @@ -152,13 +152,11 @@ static void ak4114_init_regs(struct ak4114 *chip) void snd_ak4114_reinit(struct ak4114 *chip) { - chip->init = 1; - mb(); - flush_delayed_work(&chip->work); + if (atomic_inc_return(&chip->wq_processing) == 1) + cancel_delayed_work_sync(&chip->work); ak4114_init_regs(chip); /* bring up statistics / event queing */ - chip->init = 0; - if (chip->kctls[0]) + if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } @@ -612,10 +610,10 @@ static void ak4114_stats(struct work_struct *work) { struct ak4114 *chip = container_of(work, struct ak4114, work.work); - if (!chip->init) + if (atomic_inc_return(&chip->wq_processing) == 1) snd_ak4114_check_rate_and_errors(chip, chip->check_flags); - - schedule_delayed_work(&chip->work, HZ / 10); + if (atomic_dec_and_test(&chip->wq_processing)) + schedule_delayed_work(&chip->work, HZ / 10); } EXPORT_SYMBOL(snd_ak4114_create); -- cgit v1.2.3 From a850ef8afd3cd1eecd2e136f8dca0c06599eade8 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 13 Jan 2015 10:58:31 +0100 Subject: ALSA: ak4114: Move EXPORT_SYMBOL() after each function ... just to follow the standard coding style. Acked-by: Jaroslav Kysela Tested-by: Pavel Hofman Signed-off-by: Takashi Iwai --- sound/i2c/other/ak4114.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'sound/i2c') diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c index b70e6eccbd03..f6ee1e7a9bbd 100644 --- a/sound/i2c/other/ak4114.c +++ b/sound/i2c/other/ak4114.c @@ -122,6 +122,7 @@ int snd_ak4114_create(struct snd_card *card, snd_ak4114_free(chip); return err < 0 ? err : -EIO; } +EXPORT_SYMBOL(snd_ak4114_create); void snd_ak4114_reg_write(struct ak4114 *chip, unsigned char reg, unsigned char mask, unsigned char val) { @@ -131,6 +132,7 @@ void snd_ak4114_reg_write(struct ak4114 *chip, unsigned char reg, unsigned char reg_write(chip, reg, (chip->txcsb[reg-AK4114_REG_TXCSB0] & ~mask) | val); } +EXPORT_SYMBOL(snd_ak4114_reg_write); static void ak4114_init_regs(struct ak4114 *chip) { @@ -159,6 +161,7 @@ void snd_ak4114_reinit(struct ak4114 *chip) if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } +EXPORT_SYMBOL(snd_ak4114_reinit); static unsigned int external_rate(unsigned char rcs1) { @@ -503,6 +506,7 @@ int snd_ak4114_build(struct ak4114 *ak4114, schedule_delayed_work(&ak4114->work, HZ / 10); return 0; } +EXPORT_SYMBOL(snd_ak4114_build); /* notify kcontrols if any parameters are changed */ static void ak4114_notify(struct ak4114 *ak4114, @@ -558,6 +562,7 @@ int snd_ak4114_external_rate(struct ak4114 *ak4114) rcs1 = reg_read(ak4114, AK4114_REG_RCS1); return external_rate(rcs1); } +EXPORT_SYMBOL(snd_ak4114_external_rate); int snd_ak4114_check_rate_and_errors(struct ak4114 *ak4114, unsigned int flags) { @@ -605,6 +610,7 @@ int snd_ak4114_check_rate_and_errors(struct ak4114 *ak4114, unsigned int flags) } return res; } +EXPORT_SYMBOL(snd_ak4114_check_rate_and_errors); static void ak4114_stats(struct work_struct *work) { @@ -615,10 +621,3 @@ static void ak4114_stats(struct work_struct *work) if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } - -EXPORT_SYMBOL(snd_ak4114_create); -EXPORT_SYMBOL(snd_ak4114_reg_write); -EXPORT_SYMBOL(snd_ak4114_reinit); -EXPORT_SYMBOL(snd_ak4114_build); -EXPORT_SYMBOL(snd_ak4114_external_rate); -EXPORT_SYMBOL(snd_ak4114_check_rate_and_errors); -- cgit v1.2.3 From 1293617cddc40971917150e3f5bf66b7306e2e7e Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 13 Jan 2015 11:24:08 +0100 Subject: ALSA: ak411x: Add PM helper functions Define snd_ak4114_suspend() and snd_ak4114_resume() functions to handle PM properly, stopping and restarting the work at PM. Currently only ice1712/juli.c deals with the PM and ak4114, so fix the calls there appropriately. The same PM functions are defined in ak4113.c, too, although they aren't currently called yet (ice1712/quartet.c may be enhanced to support PM later). Acked-by: Jaroslav Kysela Tested-by: Pavel Hofman Signed-off-by: Takashi Iwai --- include/sound/ak4113.h | 8 ++++++++ include/sound/ak4114.h | 8 ++++++++ sound/i2c/other/ak4113.c | 16 ++++++++++++++++ sound/i2c/other/ak4114.c | 16 ++++++++++++++++ sound/pci/ice1712/juli.c | 4 +++- 5 files changed, 51 insertions(+), 1 deletion(-) (limited to 'sound/i2c') diff --git a/include/sound/ak4113.h b/include/sound/ak4113.h index 3a34f6edc2d1..f4fbf6888e78 100644 --- a/include/sound/ak4113.h +++ b/include/sound/ak4113.h @@ -317,5 +317,13 @@ int snd_ak4113_build(struct ak4113 *ak4113, int snd_ak4113_external_rate(struct ak4113 *ak4113); int snd_ak4113_check_rate_and_errors(struct ak4113 *ak4113, unsigned int flags); +#ifdef CONFIG_PM +void snd_ak4113_suspend(struct ak4113 *chip); +void snd_ak4113_resume(struct ak4113 *chip); +#else +static inline void snd_ak4113_suspend(struct ak4113 *chip) {} +static inline void snd_ak4113_resume(struct ak4113 *chip) {} +#endif + #endif /* __SOUND_AK4113_H */ diff --git a/include/sound/ak4114.h b/include/sound/ak4114.h index 069299a88915..e681df2c95e1 100644 --- a/include/sound/ak4114.h +++ b/include/sound/ak4114.h @@ -199,5 +199,13 @@ int snd_ak4114_build(struct ak4114 *ak4114, int snd_ak4114_external_rate(struct ak4114 *ak4114); int snd_ak4114_check_rate_and_errors(struct ak4114 *ak4114, unsigned int flags); +#ifdef CONFIG_PM +void snd_ak4114_suspend(struct ak4114 *chip); +void snd_ak4114_resume(struct ak4114 *chip); +#else +static inline void snd_ak4114_suspend(struct ak4114 *chip) {} +static inline void snd_ak4114_resume(struct ak4114 *chip) {} +#endif + #endif /* __SOUND_AK4114_H */ diff --git a/sound/i2c/other/ak4113.c b/sound/i2c/other/ak4113.c index c6bba99a90b2..c2b14d981fa1 100644 --- a/sound/i2c/other/ak4113.c +++ b/sound/i2c/other/ak4113.c @@ -636,3 +636,19 @@ static void ak4113_stats(struct work_struct *work) if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } + +#ifdef CONFIG_PM +void snd_ak4113_suspend(struct ak4113 *chip) +{ + atomic_inc(&chip->wq_processing); /* don't schedule new work */ + cancel_delayed_work_sync(&chip->work); +} +EXPORT_SYMBOL(snd_ak4113_suspend); + +void snd_ak4113_resume(struct ak4113 *chip) +{ + atomic_dec(&chip->wq_processing); + snd_ak4113_reinit(chip); +} +EXPORT_SYMBOL(snd_ak4113_resume); +#endif diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c index f6ee1e7a9bbd..ee639f57b664 100644 --- a/sound/i2c/other/ak4114.c +++ b/sound/i2c/other/ak4114.c @@ -621,3 +621,19 @@ static void ak4114_stats(struct work_struct *work) if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); } + +#ifdef CONFIG_PM +void snd_ak4114_suspend(struct ak4114 *chip) +{ + atomic_inc(&chip->wq_processing); /* don't schedule new work */ + cancel_delayed_work_sync(&chip->work); +} +EXPORT_SYMBOL(snd_ak4114_suspend); + +void snd_ak4114_resume(struct ak4114 *chip) +{ + atomic_dec(&chip->wq_processing); + snd_ak4114_reinit(chip); +} +EXPORT_SYMBOL(snd_ak4114_resume); +#endif diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c index a1536c1a7ed4..4f0213427152 100644 --- a/sound/pci/ice1712/juli.c +++ b/sound/pci/ice1712/juli.c @@ -491,15 +491,17 @@ static int juli_resume(struct snd_ice1712 *ice) /* akm4358 un-reset, un-mute */ snd_akm4xxx_reset(ak, 0); /* reinit ak4114 */ - snd_ak4114_reinit(spec->ak4114); + snd_ak4114_resume(spec->ak4114); return 0; } static int juli_suspend(struct snd_ice1712 *ice) { struct snd_akm4xxx *ak = ice->akm; + struct juli_spec *spec = ice->spec; /* akm4358 reset and soft-mute */ snd_akm4xxx_reset(ak, 1); + snd_ak4114_suspend(spec->ak4114); return 0; } #endif -- cgit v1.2.3 From 1781e78c63317c04e6ae6a076acfd53236f420bc Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 16 Jan 2015 13:03:28 +0100 Subject: ALSA: ak411x: Fix race of reinit() calls Protect the call with a mutex, as this may be called in parallel (either from the PCM rate change and the clock change). Acked-by: Jaroslav Kysela Tested-by: Pavel Hofman Signed-off-by: Takashi Iwai --- include/sound/ak4113.h | 1 + include/sound/ak4114.h | 1 + sound/i2c/other/ak4113.c | 3 +++ sound/i2c/other/ak4114.c | 3 +++ 4 files changed, 8 insertions(+) (limited to 'sound/i2c') diff --git a/include/sound/ak4113.h b/include/sound/ak4113.h index f4fbf6888e78..58c145620c3c 100644 --- a/include/sound/ak4113.h +++ b/include/sound/ak4113.h @@ -287,6 +287,7 @@ struct ak4113 { ak4113_read_t *read; void *private_data; atomic_t wq_processing; + struct mutex reinit_mutex; spinlock_t lock; unsigned char regmap[AK4113_WRITABLE_REGS]; struct snd_kcontrol *kctls[AK4113_CONTROLS]; diff --git a/include/sound/ak4114.h b/include/sound/ak4114.h index e681df2c95e1..b6feb7e225f2 100644 --- a/include/sound/ak4114.h +++ b/include/sound/ak4114.h @@ -169,6 +169,7 @@ struct ak4114 { ak4114_read_t * read; void * private_data; atomic_t wq_processing; + struct mutex reinit_mutex; spinlock_t lock; unsigned char regmap[6]; unsigned char txcsb[5]; diff --git a/sound/i2c/other/ak4113.c b/sound/i2c/other/ak4113.c index c2b14d981fa1..88844881cbff 100644 --- a/sound/i2c/other/ak4113.c +++ b/sound/i2c/other/ak4113.c @@ -89,6 +89,7 @@ int snd_ak4113_create(struct snd_card *card, ak4113_read_t *read, chip->private_data = private_data; INIT_DELAYED_WORK(&chip->work, ak4113_stats); atomic_set(&chip->wq_processing, 0); + mutex_init(&chip->reinit_mutex); for (reg = 0; reg < AK4113_WRITABLE_REGS ; reg++) chip->regmap[reg] = pgm[reg]; @@ -141,7 +142,9 @@ void snd_ak4113_reinit(struct ak4113 *chip) { if (atomic_inc_return(&chip->wq_processing) == 1) cancel_delayed_work_sync(&chip->work); + mutex_lock(&chip->reinit_mutex); ak4113_init_regs(chip); + mutex_unlock(&chip->reinit_mutex); /* bring up statistics / event queing */ if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c index ee639f57b664..5a4cf3fab4ae 100644 --- a/sound/i2c/other/ak4114.c +++ b/sound/i2c/other/ak4114.c @@ -100,6 +100,7 @@ int snd_ak4114_create(struct snd_card *card, chip->private_data = private_data; INIT_DELAYED_WORK(&chip->work, ak4114_stats); atomic_set(&chip->wq_processing, 0); + mutex_init(&chip->reinit_mutex); for (reg = 0; reg < 6; reg++) chip->regmap[reg] = pgm[reg]; @@ -156,7 +157,9 @@ void snd_ak4114_reinit(struct ak4114 *chip) { if (atomic_inc_return(&chip->wq_processing) == 1) cancel_delayed_work_sync(&chip->work); + mutex_lock(&chip->reinit_mutex); ak4114_init_regs(chip); + mutex_unlock(&chip->reinit_mutex); /* bring up statistics / event queing */ if (atomic_dec_and_test(&chip->wq_processing)) schedule_delayed_work(&chip->work, HZ / 10); -- cgit v1.2.3