[alsa-devel] [PATCH 0/7] ASoC: Cleanup AC'97 reset handling
AC'97 devices require that they are properly reset before they can be used. Currently ASoC drivers do this by hand each on its own. This lead to many similar but slightly different implementations. E.g. Some supporting warm reset, some don't. And even though AC'97 do have a device ID which uniquely identifies a device most drivers only use the content of register 0 for device verification.
This patch series introduces a new generic helper function in the ALSA AC'97 core that can be used to reset a device and verify its ID if required. And then converts drivers to use this new helper function instead of custom code.
snd_soc_new_ac97_codec() is extended to be able to perform a reset between the allocation and registration of the AC'97 device, this will allow more drivers to use it.
- Lars
Lars-Peter Clausen (7): ALSA: ac97: Add helper function to reset the AC97 device ASoC: ac97: Add support for resetting device before registration ASoC: ad1980: Use core AC'97 reset helper ASoC: stac9766: Use core reset helper ASoC: wm9705: Use core AC'97 reset helper ASoC: wm9712: Use core AC'97 reset helper ASoC: wm9713: Use core AC'97 reset helper
include/sound/ac97_codec.h | 2 ++ include/sound/soc.h | 3 ++- sound/ac97_bus.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ sound/soc/codecs/ad1980.c | 36 +++++++++----------------- sound/soc/codecs/stac9766.c | 57 +++++------------------------------------ sound/soc/codecs/wm9705.c | 40 ++++++----------------------- sound/soc/codecs/wm9712.c | 45 ++++++-------------------------- sound/soc/codecs/wm9713.c | 48 ++++++----------------------------- sound/soc/codecs/wm9713.h | 2 -- sound/soc/soc-ac97.c | 30 ++++++++++++++++++---- 10 files changed, 134 insertions(+), 191 deletions(-)
There is currently a lot of code duplication in ASoC drivers regarding the reset handling of devices. This patch introduces a new generic reset function in the generic AC'97 framework that can be used to replace most the custom reset functions.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/ac97_codec.h | 2 ++ sound/ac97_bus.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 0e9d75b..74bc8547 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -584,6 +584,8 @@ static inline int snd_ac97_update_power(struct snd_ac97 *ac97, int reg, void snd_ac97_suspend(struct snd_ac97 *ac97); void snd_ac97_resume(struct snd_ac97 *ac97); #endif +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, + unsigned int id_mask);
/* quirk types */ enum { diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c index 2b50cbe..55791a0 100644 --- a/sound/ac97_bus.c +++ b/sound/ac97_bus.c @@ -18,6 +18,68 @@ #include <sound/ac97_codec.h>
/* + * snd_ac97_check_id() - Reads and checks the vendor ID of the device + * @ac97: The AC97 device to check + * @id: The ID to compare to + * @id_mask: Mask that is applied to the device ID before comparing to @id + * + * If @id is 0 this function returns true if the read device vendor ID is + * a valid ID. If @id is non 0 this functions returns true if @id + * matches the read vendor ID. Otherwise the function returns false. + */ +static bool snd_ac97_check_id(struct snd_ac97 *ac97, unsigned int id, + unsigned int id_mask) +{ + ac97->id = ac97->bus->ops->read(ac97, AC97_VENDOR_ID1) << 16; + ac97->id |= ac97->bus->ops->read(ac97, AC97_VENDOR_ID2); + + if (ac97->id == 0x0 || ac97->id == 0xffffffff) + return false; + + if (id != 0 && id != (ac97->id & id_mask)) + return false; + + return true; +} + +/** + * snd_ac97_reset() - Reset AC'97 device + * @ac97: The AC'97 device to reset + * @try_warm: Try a warm reset first + * @id: Expected device vendor ID + * @id_mask: Mask that is applied to the device ID before comparing to @id + * + * This function resets the AC'97 device. If @try_warm is true the function + * first performs a warm reset. If the warm reset is successful the function + * returns 1. Otherwise or if @try_warm is false the function issues cold reset + * followed by a warm reset. If this is successful the function returns 0, + * otherwise a negative error code. If @id is 0 any valid device ID will be + * accepted, otherwise only the ID that matches @id and @id_mask is accepted. + */ +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, + unsigned int id_mask) +{ + struct snd_ac97_bus_ops *ops = ac97->bus->ops; + + if (try_warm && ops->warm_reset) { + ops->warm_reset(ac97); + if (snd_ac97_check_id(ac97, id, id_mask)) + return 1; + } + + if (ops->reset) + ops->reset(ac97); + if (ops->warm_reset) + ops->warm_reset(ac97); + + if (snd_ac97_check_id(ac97, id, id_mask)) + return 0; + + return -ENODEV; +} +EXPORT_SYMBOL_GPL(snd_ac97_reset); + +/* * Let drivers decide whether they want to support given codec from their * probe method. Drivers have direct access to the struct snd_ac97 * structure and may decide based on the id field amongst other things.
On Tue, 21 Jul 2015 21:53:00 +0200, Lars-Peter Clausen wrote:
There is currently a lot of code duplication in ASoC drivers regarding the reset handling of devices. This patch introduces a new generic reset function in the generic AC'97 framework that can be used to replace most the custom reset functions.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Reviewed-by: Takashi Iwai tiwai@suse.de
One possible improvement would be to allow to pass id_mask=0 as the full bit, so that you don't have to define the mask 0xffffffff at each time.
thanks,
Takashi
include/sound/ac97_codec.h | 2 ++ sound/ac97_bus.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 0e9d75b..74bc8547 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -584,6 +584,8 @@ static inline int snd_ac97_update_power(struct snd_ac97 *ac97, int reg, void snd_ac97_suspend(struct snd_ac97 *ac97); void snd_ac97_resume(struct snd_ac97 *ac97); #endif +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id,
- unsigned int id_mask);
/* quirk types */ enum { diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c index 2b50cbe..55791a0 100644 --- a/sound/ac97_bus.c +++ b/sound/ac97_bus.c @@ -18,6 +18,68 @@ #include <sound/ac97_codec.h>
/*
- snd_ac97_check_id() - Reads and checks the vendor ID of the device
- @ac97: The AC97 device to check
- @id: The ID to compare to
- @id_mask: Mask that is applied to the device ID before comparing to @id
- If @id is 0 this function returns true if the read device vendor ID is
- a valid ID. If @id is non 0 this functions returns true if @id
- matches the read vendor ID. Otherwise the function returns false.
- */
+static bool snd_ac97_check_id(struct snd_ac97 *ac97, unsigned int id,
- unsigned int id_mask)
+{
- ac97->id = ac97->bus->ops->read(ac97, AC97_VENDOR_ID1) << 16;
- ac97->id |= ac97->bus->ops->read(ac97, AC97_VENDOR_ID2);
- if (ac97->id == 0x0 || ac97->id == 0xffffffff)
return false;- if (id != 0 && id != (ac97->id & id_mask))
return false;- return true;
+}
+/**
- snd_ac97_reset() - Reset AC'97 device
- @ac97: The AC'97 device to reset
- @try_warm: Try a warm reset first
- @id: Expected device vendor ID
- @id_mask: Mask that is applied to the device ID before comparing to @id
- This function resets the AC'97 device. If @try_warm is true the function
- first performs a warm reset. If the warm reset is successful the function
- returns 1. Otherwise or if @try_warm is false the function issues cold reset
- followed by a warm reset. If this is successful the function returns 0,
- otherwise a negative error code. If @id is 0 any valid device ID will be
- accepted, otherwise only the ID that matches @id and @id_mask is accepted.
- */
+int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id,
- unsigned int id_mask)
+{
- struct snd_ac97_bus_ops *ops = ac97->bus->ops;
- if (try_warm && ops->warm_reset) {
ops->warm_reset(ac97);if (snd_ac97_check_id(ac97, id, id_mask))return 1;- }
- if (ops->reset)
ops->reset(ac97);- if (ops->warm_reset)
ops->warm_reset(ac97);- if (snd_ac97_check_id(ac97, id, id_mask))
return 0;- return -ENODEV;
+} +EXPORT_SYMBOL_GPL(snd_ac97_reset);
+/*
- Let drivers decide whether they want to support given codec from their
- probe method. Drivers have direct access to the struct snd_ac97
- structure and may decide based on the id field amongst other things.
-- 2.1.4
The patch
ALSA: ac97: Add helper function to reset the AC97 device
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 5f1d980ee9b6353f18765bfa6774a5a08d6cb944 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:00 +0200 Subject: [PATCH] ALSA: ac97: Add helper function to reset the AC97 device
There is currently a lot of code duplication in ASoC drivers regarding the reset handling of devices. This patch introduces a new generic reset function in the generic AC'97 framework that can be used to replace most the custom reset functions.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/ac97_codec.h | 2 ++ sound/ac97_bus.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 0e9d75b49bed..74bc85473b58 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -584,6 +584,8 @@ static inline int snd_ac97_update_power(struct snd_ac97 *ac97, int reg, void snd_ac97_suspend(struct snd_ac97 *ac97); void snd_ac97_resume(struct snd_ac97 *ac97); #endif +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, + unsigned int id_mask);
/* quirk types */ enum { diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c index 2b50cbe6aca9..55791a0b3943 100644 --- a/sound/ac97_bus.c +++ b/sound/ac97_bus.c @@ -18,6 +18,68 @@ #include <sound/ac97_codec.h>
/* + * snd_ac97_check_id() - Reads and checks the vendor ID of the device + * @ac97: The AC97 device to check + * @id: The ID to compare to + * @id_mask: Mask that is applied to the device ID before comparing to @id + * + * If @id is 0 this function returns true if the read device vendor ID is + * a valid ID. If @id is non 0 this functions returns true if @id + * matches the read vendor ID. Otherwise the function returns false. + */ +static bool snd_ac97_check_id(struct snd_ac97 *ac97, unsigned int id, + unsigned int id_mask) +{ + ac97->id = ac97->bus->ops->read(ac97, AC97_VENDOR_ID1) << 16; + ac97->id |= ac97->bus->ops->read(ac97, AC97_VENDOR_ID2); + + if (ac97->id == 0x0 || ac97->id == 0xffffffff) + return false; + + if (id != 0 && id != (ac97->id & id_mask)) + return false; + + return true; +} + +/** + * snd_ac97_reset() - Reset AC'97 device + * @ac97: The AC'97 device to reset + * @try_warm: Try a warm reset first + * @id: Expected device vendor ID + * @id_mask: Mask that is applied to the device ID before comparing to @id + * + * This function resets the AC'97 device. If @try_warm is true the function + * first performs a warm reset. If the warm reset is successful the function + * returns 1. Otherwise or if @try_warm is false the function issues cold reset + * followed by a warm reset. If this is successful the function returns 0, + * otherwise a negative error code. If @id is 0 any valid device ID will be + * accepted, otherwise only the ID that matches @id and @id_mask is accepted. + */ +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, + unsigned int id_mask) +{ + struct snd_ac97_bus_ops *ops = ac97->bus->ops; + + if (try_warm && ops->warm_reset) { + ops->warm_reset(ac97); + if (snd_ac97_check_id(ac97, id, id_mask)) + return 1; + } + + if (ops->reset) + ops->reset(ac97); + if (ops->warm_reset) + ops->warm_reset(ac97); + + if (snd_ac97_check_id(ac97, id, id_mask)) + return 0; + + return -ENODEV; +} +EXPORT_SYMBOL_GPL(snd_ac97_reset); + +/* * Let drivers decide whether they want to support given codec from their * probe method. Drivers have direct access to the struct snd_ac97 * structure and may decide based on the id field amongst other things.
AC97 devices need to be initially reset before they can be used. Currently each driver does this on its own.
Add support for resetting the device to core in snd_soc_new_ac97_codec(). If the caller supplies a device ID and device ID mask the function will reset the device and verify that it has the correct ID, if it does not a error is returned.
This will allow to remove custom code with similar functionality from individual drivers.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc.h | 3 ++- sound/soc/codecs/ad1980.c | 2 +- sound/soc/codecs/stac9766.c | 2 +- sound/soc/soc-ac97.c | 30 +++++++++++++++++++++++++----- 4 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index add5097..4cef20e 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -526,7 +526,8 @@ int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned int reg,
#ifdef CONFIG_SND_SOC_AC97_BUS struct snd_ac97 *snd_soc_alloc_ac97_codec(struct snd_soc_codec *codec); -struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec); +struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec, + unsigned int id, unsigned int id_mask); void snd_soc_free_ac97_codec(struct snd_ac97 *ac97);
int snd_soc_set_ac97_ops(struct snd_ac97_bus_ops *ops); diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c index 3cc69a6..d9cb81d 100644 --- a/sound/soc/codecs/ad1980.c +++ b/sound/soc/codecs/ad1980.c @@ -240,7 +240,7 @@ static int ad1980_soc_probe(struct snd_soc_codec *codec) u16 vendor_id2; u16 ext_status;
- ac97 = snd_soc_new_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, 0, 0); if (IS_ERR(ac97)) { ret = PTR_ERR(ac97); dev_err(codec->dev, "Failed to register AC97 codec: %d\n", ret); diff --git a/sound/soc/codecs/stac9766.c b/sound/soc/codecs/stac9766.c index ed4cca7..c602830 100644 --- a/sound/soc/codecs/stac9766.c +++ b/sound/soc/codecs/stac9766.c @@ -332,7 +332,7 @@ static int stac9766_codec_probe(struct snd_soc_codec *codec) struct snd_ac97 *ac97; int ret = 0;
- ac97 = snd_soc_new_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, 0, 0); if (IS_ERR(ac97)) return PTR_ERR(ac97);
diff --git a/sound/soc/soc-ac97.c b/sound/soc/soc-ac97.c index 08d7259..d40efc9 100644 --- a/sound/soc/soc-ac97.c +++ b/sound/soc/soc-ac97.c @@ -85,10 +85,19 @@ EXPORT_SYMBOL(snd_soc_alloc_ac97_codec); /** * snd_soc_new_ac97_codec - initailise AC97 device * @codec: audio codec + * @id: The expected device ID + * @id_mask: Mask that is applied to the device ID before comparing with @id * * Initialises AC97 codec resources for use by ad-hoc devices only. + * + * If @id is not 0 this function will reset the device, then read the ID from + * the device and check if it matches the expected ID. If it doesn't match an + * error will be returned and device will not be registered. + * + * Returns: A PTR_ERR() on failure or a valid snd_ac97 struct on success. */ -struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec) +struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec, + unsigned int id, unsigned int id_mask) { struct snd_ac97 *ac97; int ret; @@ -97,13 +106,24 @@ struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec) if (IS_ERR(ac97)) return ac97;
- ret = device_add(&ac97->dev); - if (ret) { - put_device(&ac97->dev); - return ERR_PTR(ret); + if (id) { + ret = snd_ac97_reset(ac97, false, id, id_mask); + if (ret < 0) { + dev_err(codec->dev, "Failed to reset AC97 device: %d\n", + ret); + goto err_put_device; + } }
+ ret = device_add(&ac97->dev); + if (ret) + goto err_put_device; + return ac97; + +err_put_device: + put_device(&ac97->dev); + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(snd_soc_new_ac97_codec);
The patch
ASoC: ac97: Add support for resetting device before registration
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 7361fbeaeaab5282bbfc88f1f6fe4cf034f7623c Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:01 +0200 Subject: [PATCH] ASoC: ac97: Add support for resetting device before registration
AC97 devices need to be initially reset before they can be used. Currently each driver does this on its own.
Add support for resetting the device to core in snd_soc_new_ac97_codec(). If the caller supplies a device ID and device ID mask the function will reset the device and verify that it has the correct ID, if it does not a error is returned.
This will allow to remove custom code with similar functionality from individual drivers.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 3 ++- sound/soc/codecs/ad1980.c | 2 +- sound/soc/codecs/stac9766.c | 2 +- sound/soc/soc-ac97.c | 30 +++++++++++++++++++++++++----- 4 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 93df8bf9d54a..42d144a4b7ba 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -526,7 +526,8 @@ int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned int reg,
#ifdef CONFIG_SND_SOC_AC97_BUS struct snd_ac97 *snd_soc_alloc_ac97_codec(struct snd_soc_codec *codec); -struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec); +struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec, + unsigned int id, unsigned int id_mask); void snd_soc_free_ac97_codec(struct snd_ac97 *ac97);
int snd_soc_set_ac97_ops(struct snd_ac97_bus_ops *ops); diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c index 3cc69a626454..d9cb81dd64b5 100644 --- a/sound/soc/codecs/ad1980.c +++ b/sound/soc/codecs/ad1980.c @@ -240,7 +240,7 @@ static int ad1980_soc_probe(struct snd_soc_codec *codec) u16 vendor_id2; u16 ext_status;
- ac97 = snd_soc_new_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, 0, 0); if (IS_ERR(ac97)) { ret = PTR_ERR(ac97); dev_err(codec->dev, "Failed to register AC97 codec: %d\n", ret); diff --git a/sound/soc/codecs/stac9766.c b/sound/soc/codecs/stac9766.c index ed4cca7f6779..c6028300c0ac 100644 --- a/sound/soc/codecs/stac9766.c +++ b/sound/soc/codecs/stac9766.c @@ -332,7 +332,7 @@ static int stac9766_codec_probe(struct snd_soc_codec *codec) struct snd_ac97 *ac97; int ret = 0;
- ac97 = snd_soc_new_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, 0, 0); if (IS_ERR(ac97)) return PTR_ERR(ac97);
diff --git a/sound/soc/soc-ac97.c b/sound/soc/soc-ac97.c index 08d7259bbaab..d40efc9fe0a9 100644 --- a/sound/soc/soc-ac97.c +++ b/sound/soc/soc-ac97.c @@ -85,10 +85,19 @@ EXPORT_SYMBOL(snd_soc_alloc_ac97_codec); /** * snd_soc_new_ac97_codec - initailise AC97 device * @codec: audio codec + * @id: The expected device ID + * @id_mask: Mask that is applied to the device ID before comparing with @id * * Initialises AC97 codec resources for use by ad-hoc devices only. + * + * If @id is not 0 this function will reset the device, then read the ID from + * the device and check if it matches the expected ID. If it doesn't match an + * error will be returned and device will not be registered. + * + * Returns: A PTR_ERR() on failure or a valid snd_ac97 struct on success. */ -struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec) +struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec, + unsigned int id, unsigned int id_mask) { struct snd_ac97 *ac97; int ret; @@ -97,13 +106,24 @@ struct snd_ac97 *snd_soc_new_ac97_codec(struct snd_soc_codec *codec) if (IS_ERR(ac97)) return ac97;
- ret = device_add(&ac97->dev); - if (ret) { - put_device(&ac97->dev); - return ERR_PTR(ret); + if (id) { + ret = snd_ac97_reset(ac97, false, id, id_mask); + if (ret < 0) { + dev_err(codec->dev, "Failed to reset AC97 device: %d\n", + ret); + goto err_put_device; + } }
+ ret = device_add(&ac97->dev); + if (ret) + goto err_put_device; + return ac97; + +err_put_device: + put_device(&ac97->dev); + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(snd_soc_new_ac97_codec);
Use the new snd_ac97_reset() helper function to perform the reset and verify the device ID.
Unfortunately the reset can't be done in snd_soc_new_ac97_codec() due to the special requirements in order to support the non-standard 16-bit slot mode of the ad1980.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/ad1980.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c index d9cb81d..9ef20db 100644 --- a/sound/soc/codecs/ad1980.c +++ b/sound/soc/codecs/ad1980.c @@ -202,19 +202,21 @@ static struct snd_soc_dai_driver ad1980_dai = { .formats = SND_SOC_STD_AC97_FMTS, }, };
+#define AD1980_VENDOR_ID 0x41445300 +#define AD1980_VENDOR_MASK 0xffffff00 + static int ad1980_reset(struct snd_soc_codec *codec, int try_warm) { struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); unsigned int retry_cnt = 0; + int ret;
do { - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(ac97); - if (snd_soc_read(codec, AC97_RESET) == 0x0090) - return 1; - } + ret = snd_ac97_reset(ac97, true, AD1980_VENDOR_ID, + AD1980_VENDOR_MASK); + if (ret >= 0) + return 0;
- soc_ac97_ops->reset(ac97); /* * Set bit 16slot in register 74h, then every slot will has only * 16 bits. This command is sent out in 20bit mode, in which @@ -223,8 +225,6 @@ static int ad1980_reset(struct snd_soc_codec *codec, int try_warm) */ snd_soc_write(codec, AC97_AD_SERIAL_CFG, 0x9900);
- if (snd_soc_read(codec, AC97_RESET) == 0x0090) - return 0; } while (retry_cnt++ < 10);
dev_err(codec->dev, "Failed to reset: AC97 link error\n"); @@ -260,22 +260,10 @@ static int ad1980_soc_probe(struct snd_soc_codec *codec) if (ret < 0) goto reset_err;
- /* Read out vendor ID to make sure it is ad1980 */ - if (snd_soc_read(codec, AC97_VENDOR_ID1) != 0x4144) { - ret = -ENODEV; - goto reset_err; - } - vendor_id2 = snd_soc_read(codec, AC97_VENDOR_ID2); - - if (vendor_id2 != 0x5370) { - if (vendor_id2 != 0x5374) { - ret = -ENODEV; - goto reset_err; - } else { - dev_warn(codec->dev, - "Found AD1981 - only 2/2 IN/OUT Channels supported\n"); - } + if (vendor_id2 == 0x5374) { + dev_warn(codec->dev, + "Found AD1981 - only 2/2 IN/OUT Channels supported\n"); }
/* unmute captures and playbacks volume */
The patch
ASoC: ad1980: Use core AC'97 reset helper
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 3ab3dbdfb91b70ef6bf4eb9b544bf54ff1dff51a Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:02 +0200 Subject: [PATCH] ASoC: ad1980: Use core AC'97 reset helper
Use the new snd_ac97_reset() helper function to perform the reset and verify the device ID.
Unfortunately the reset can't be done in snd_soc_new_ac97_codec() due to the special requirements in order to support the non-standard 16-bit slot mode of the ad1980.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/ad1980.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c index d9cb81dd64b5..9ef20dbccbe3 100644 --- a/sound/soc/codecs/ad1980.c +++ b/sound/soc/codecs/ad1980.c @@ -202,19 +202,21 @@ static struct snd_soc_dai_driver ad1980_dai = { .formats = SND_SOC_STD_AC97_FMTS, }, };
+#define AD1980_VENDOR_ID 0x41445300 +#define AD1980_VENDOR_MASK 0xffffff00 + static int ad1980_reset(struct snd_soc_codec *codec, int try_warm) { struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); unsigned int retry_cnt = 0; + int ret;
do { - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(ac97); - if (snd_soc_read(codec, AC97_RESET) == 0x0090) - return 1; - } + ret = snd_ac97_reset(ac97, true, AD1980_VENDOR_ID, + AD1980_VENDOR_MASK); + if (ret >= 0) + return 0;
- soc_ac97_ops->reset(ac97); /* * Set bit 16slot in register 74h, then every slot will has only * 16 bits. This command is sent out in 20bit mode, in which @@ -223,8 +225,6 @@ static int ad1980_reset(struct snd_soc_codec *codec, int try_warm) */ snd_soc_write(codec, AC97_AD_SERIAL_CFG, 0x9900);
- if (snd_soc_read(codec, AC97_RESET) == 0x0090) - return 0; } while (retry_cnt++ < 10);
dev_err(codec->dev, "Failed to reset: AC97 link error\n"); @@ -260,22 +260,10 @@ static int ad1980_soc_probe(struct snd_soc_codec *codec) if (ret < 0) goto reset_err;
- /* Read out vendor ID to make sure it is ad1980 */ - if (snd_soc_read(codec, AC97_VENDOR_ID1) != 0x4144) { - ret = -ENODEV; - goto reset_err; - } - vendor_id2 = snd_soc_read(codec, AC97_VENDOR_ID2); - - if (vendor_id2 != 0x5370) { - if (vendor_id2 != 0x5374) { - ret = -ENODEV; - goto reset_err; - } else { - dev_warn(codec->dev, - "Found AD1981 - only 2/2 IN/OUT Channels supported\n"); - } + if (vendor_id2 == 0x5374) { + dev_warn(codec->dev, + "Found AD1981 - only 2/2 IN/OUT Channels supported\n"); }
/* unmute captures and playbacks volume */
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/stac9766.c | 57 ++++++--------------------------------------- 1 file changed, 7 insertions(+), 50 deletions(-)
diff --git a/sound/soc/codecs/stac9766.c b/sound/soc/codecs/stac9766.c index c602830..0945c51 100644 --- a/sound/soc/codecs/stac9766.c +++ b/sound/soc/codecs/stac9766.c @@ -28,6 +28,9 @@
#include "stac9766.h"
+#define STAC9766_VENDOR_ID 0x83847666 +#define STAC9766_VENDOR_ID_MASK 0xffffffff + /* * STAC9766 register cache */ @@ -239,45 +242,12 @@ static int stac9766_set_bias_level(struct snd_soc_codec *codec, return 0; }
-static int stac9766_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(ac97); - if (stac9766_ac97_read(codec, 0) == stac9766_reg[0]) - return 1; - } - - soc_ac97_ops->reset(ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(ac97); - if (stac9766_ac97_read(codec, 0) != stac9766_reg[0]) - return -EIO; - return 0; -} - static int stac9766_codec_resume(struct snd_soc_codec *codec) { struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - u16 id, reset;
- reset = 0; - /* give the codec an AC97 warm reset to start the link */ -reset: - if (reset > 5) { - dev_err(codec->dev, "Failed to resume\n"); - return -EIO; - } - ac97->bus->ops->warm_reset(ac97); - id = soc_ac97_ops->read(ac97, AC97_VENDOR_ID2); - if (id != 0x4c13) { - stac9766_reset(codec, 0); - reset++; - goto reset; - } - - return 0; + return snd_ac97_reset(ac97, true, STAC9766_VENDOR_ID, + STAC9766_VENDOR_ID_MASK); }
static const struct snd_soc_dai_ops stac9766_dai_ops_analog = { @@ -330,28 +300,15 @@ static struct snd_soc_dai_driver stac9766_dai[] = { static int stac9766_codec_probe(struct snd_soc_codec *codec) { struct snd_ac97 *ac97; - int ret = 0;
- ac97 = snd_soc_new_ac97_codec(codec, 0, 0); + ac97 = snd_soc_new_ac97_codec(codec, STAC9766_VENDOR_ID, + STAC9766_VENDOR_ID_MASK); if (IS_ERR(ac97)) return PTR_ERR(ac97);
snd_soc_codec_set_drvdata(codec, ac97);
- /* do a cold reset for the controller and then try - * a warm reset followed by an optional cold reset for codec */ - stac9766_reset(codec, 0); - ret = stac9766_reset(codec, 1); - if (ret < 0) { - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - goto codec_err; - } - return 0; - -codec_err: - snd_soc_free_ac97_codec(ac97); - return ret; }
static int stac9766_codec_remove(struct snd_soc_codec *codec)
The patch
ASoC: stac9766: Use core reset helper
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 017e800af9f91861dcd6e4fd8a29418de86fd884 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:03 +0200 Subject: [PATCH] ASoC: stac9766: Use core reset helper
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/stac9766.c | 57 ++++++--------------------------------------- 1 file changed, 7 insertions(+), 50 deletions(-)
diff --git a/sound/soc/codecs/stac9766.c b/sound/soc/codecs/stac9766.c index c6028300c0ac..0945c51df003 100644 --- a/sound/soc/codecs/stac9766.c +++ b/sound/soc/codecs/stac9766.c @@ -28,6 +28,9 @@
#include "stac9766.h"
+#define STAC9766_VENDOR_ID 0x83847666 +#define STAC9766_VENDOR_ID_MASK 0xffffffff + /* * STAC9766 register cache */ @@ -239,45 +242,12 @@ static int stac9766_set_bias_level(struct snd_soc_codec *codec, return 0; }
-static int stac9766_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(ac97); - if (stac9766_ac97_read(codec, 0) == stac9766_reg[0]) - return 1; - } - - soc_ac97_ops->reset(ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(ac97); - if (stac9766_ac97_read(codec, 0) != stac9766_reg[0]) - return -EIO; - return 0; -} - static int stac9766_codec_resume(struct snd_soc_codec *codec) { struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - u16 id, reset;
- reset = 0; - /* give the codec an AC97 warm reset to start the link */ -reset: - if (reset > 5) { - dev_err(codec->dev, "Failed to resume\n"); - return -EIO; - } - ac97->bus->ops->warm_reset(ac97); - id = soc_ac97_ops->read(ac97, AC97_VENDOR_ID2); - if (id != 0x4c13) { - stac9766_reset(codec, 0); - reset++; - goto reset; - } - - return 0; + return snd_ac97_reset(ac97, true, STAC9766_VENDOR_ID, + STAC9766_VENDOR_ID_MASK); }
static const struct snd_soc_dai_ops stac9766_dai_ops_analog = { @@ -330,28 +300,15 @@ static struct snd_soc_dai_driver stac9766_dai[] = { static int stac9766_codec_probe(struct snd_soc_codec *codec) { struct snd_ac97 *ac97; - int ret = 0;
- ac97 = snd_soc_new_ac97_codec(codec, 0, 0); + ac97 = snd_soc_new_ac97_codec(codec, STAC9766_VENDOR_ID, + STAC9766_VENDOR_ID_MASK); if (IS_ERR(ac97)) return PTR_ERR(ac97);
snd_soc_codec_set_drvdata(codec, ac97);
- /* do a cold reset for the controller and then try - * a warm reset followed by an optional cold reset for codec */ - stac9766_reset(codec, 0); - ret = stac9766_reset(codec, 1); - if (ret < 0) { - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - goto codec_err; - } - return 0; - -codec_err: - snd_soc_free_ac97_codec(ac97); - return ret; }
static int stac9766_codec_remove(struct snd_soc_codec *codec)
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/wm9705.c | 40 ++++++++-------------------------------- 1 file changed, 8 insertions(+), 32 deletions(-)
diff --git a/sound/soc/codecs/wm9705.c b/sound/soc/codecs/wm9705.c index 5cc457e..744842c 100644 --- a/sound/soc/codecs/wm9705.c +++ b/sound/soc/codecs/wm9705.c @@ -22,6 +22,9 @@
#include "wm9705.h"
+#define WM9705_VENDOR_ID 0x574d4c05 +#define WM9705_VENDOR_ID_MASK 0xffffffff + /* * WM9705 register cache */ @@ -293,21 +296,6 @@ static struct snd_soc_dai_driver wm9705_dai[] = { } };
-static int wm9705_reset(struct snd_soc_codec *codec) -{ - struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - - if (soc_ac97_ops->reset) { - soc_ac97_ops->reset(ac97); - if (ac97_read(codec, 0) == wm9705_reg[0]) - return 0; /* Success */ - } - - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - - return -EIO; -} - #ifdef CONFIG_PM static int wm9705_soc_suspend(struct snd_soc_codec *codec) { @@ -324,7 +312,8 @@ static int wm9705_soc_resume(struct snd_soc_codec *codec) int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9705_reset(codec); + ret = snd_ac97_reset(ac97, true, WM9705_VENDOR_ID, + WM9705_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -342,30 +331,17 @@ static int wm9705_soc_resume(struct snd_soc_codec *codec) static int wm9705_soc_probe(struct snd_soc_codec *codec) { struct snd_ac97 *ac97; - int ret = 0;
- ac97 = snd_soc_alloc_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, WM9705_VENDOR_ID, + WM9705_VENDOR_ID_MASK); if (IS_ERR(ac97)) { - ret = PTR_ERR(ac97); dev_err(codec->dev, "Failed to register AC97 codec\n"); - return ret; + return PTR_ERR(ac97); }
- ret = wm9705_reset(codec); - if (ret) - goto err_put_device; - - ret = device_add(&ac97->dev); - if (ret) - goto err_put_device; - snd_soc_codec_set_drvdata(codec, ac97);
return 0; - -err_put_device: - put_device(&ac97->dev); - return ret; }
static int wm9705_soc_remove(struct snd_soc_codec *codec)
The patch
ASoC: wm9705: Use core AC'97 reset helper
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 6e0b73a0a172f4d881092e388b3a3ad57ca80107 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:04 +0200 Subject: [PATCH] ASoC: wm9705: Use core AC'97 reset helper
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm9705.c | 40 ++++++++-------------------------------- 1 file changed, 8 insertions(+), 32 deletions(-)
diff --git a/sound/soc/codecs/wm9705.c b/sound/soc/codecs/wm9705.c index 5cc457ef8894..744842c76a60 100644 --- a/sound/soc/codecs/wm9705.c +++ b/sound/soc/codecs/wm9705.c @@ -22,6 +22,9 @@
#include "wm9705.h"
+#define WM9705_VENDOR_ID 0x574d4c05 +#define WM9705_VENDOR_ID_MASK 0xffffffff + /* * WM9705 register cache */ @@ -293,21 +296,6 @@ static struct snd_soc_dai_driver wm9705_dai[] = { } };
-static int wm9705_reset(struct snd_soc_codec *codec) -{ - struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec); - - if (soc_ac97_ops->reset) { - soc_ac97_ops->reset(ac97); - if (ac97_read(codec, 0) == wm9705_reg[0]) - return 0; /* Success */ - } - - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - - return -EIO; -} - #ifdef CONFIG_PM static int wm9705_soc_suspend(struct snd_soc_codec *codec) { @@ -324,7 +312,8 @@ static int wm9705_soc_resume(struct snd_soc_codec *codec) int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9705_reset(codec); + ret = snd_ac97_reset(ac97, true, WM9705_VENDOR_ID, + WM9705_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -342,30 +331,17 @@ static int wm9705_soc_resume(struct snd_soc_codec *codec) static int wm9705_soc_probe(struct snd_soc_codec *codec) { struct snd_ac97 *ac97; - int ret = 0;
- ac97 = snd_soc_alloc_ac97_codec(codec); + ac97 = snd_soc_new_ac97_codec(codec, WM9705_VENDOR_ID, + WM9705_VENDOR_ID_MASK); if (IS_ERR(ac97)) { - ret = PTR_ERR(ac97); dev_err(codec->dev, "Failed to register AC97 codec\n"); - return ret; + return PTR_ERR(ac97); }
- ret = wm9705_reset(codec); - if (ret) - goto err_put_device; - - ret = device_add(&ac97->dev); - if (ret) - goto err_put_device; - snd_soc_codec_set_drvdata(codec, ac97);
return 0; - -err_put_device: - put_device(&ac97->dev); - return ret; }
static int wm9705_soc_remove(struct snd_soc_codec *codec)
On Tue, Jul 21, 2015 at 09:53:04PM +0200, Lars-Peter Clausen wrote:
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Acked-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Thanks, Charles
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/wm9712.c | 45 ++++++++------------------------------------- 1 file changed, 8 insertions(+), 37 deletions(-)
diff --git a/sound/soc/codecs/wm9712.c b/sound/soc/codecs/wm9712.c index 1fda104..488a922 100644 --- a/sound/soc/codecs/wm9712.c +++ b/sound/soc/codecs/wm9712.c @@ -23,6 +23,9 @@ #include <sound/tlv.h> #include "wm9712.h"
+#define WM9712_VENDOR_ID 0x574d4c12 +#define WM9712_VENDOR_ID_MASK 0xffffffff + struct wm9712_priv { struct snd_ac97 *ac97; unsigned int hp_mixer[2]; @@ -613,35 +616,14 @@ static int wm9712_set_bias_level(struct snd_soc_codec *codec, return 0; }
-static int wm9712_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(wm9712->ac97); - if (ac97_read(codec, 0) == wm9712_reg[0]) - return 1; - } - - soc_ac97_ops->reset(wm9712->ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(wm9712->ac97); - if (ac97_read(codec, 0) != wm9712_reg[0]) - goto err; - return 0; - -err: - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - return -EIO; -} - static int wm9712_soc_resume(struct snd_soc_codec *codec) { struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9712_reset(codec, 1); + ret = snd_ac97_reset(wm9712->ac97, true, WM9712_VENDOR_ID, + WM9712_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -663,31 +645,20 @@ static int wm9712_soc_resume(struct snd_soc_codec *codec) static int wm9712_soc_probe(struct snd_soc_codec *codec) { struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); - int ret = 0; + int ret;
- wm9712->ac97 = snd_soc_alloc_ac97_codec(codec); + wm9712->ac97 = snd_soc_new_ac97_codec(codec, WM9712_VENDOR_ID, + WM9712_VENDOR_ID_MASK); if (IS_ERR(wm9712->ac97)) { ret = PTR_ERR(wm9712->ac97); dev_err(codec->dev, "Failed to register AC97 codec: %d\n", ret); return ret; }
- ret = wm9712_reset(codec, 0); - if (ret < 0) - goto err_put_device; - - ret = device_add(&wm9712->ac97->dev); - if (ret) - goto err_put_device; - /* set alc mux to none */ ac97_write(codec, AC97_VIDEO, ac97_read(codec, AC97_VIDEO) | 0x3000);
return 0; - -err_put_device: - put_device(&wm9712->ac97->dev); - return ret; }
static int wm9712_soc_remove(struct snd_soc_codec *codec)
The patch
ASoC: wm9712: Use core AC'97 reset helper
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From a575be4cbb951244f93342487c2537f729f2239d Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:05 +0200 Subject: [PATCH] ASoC: wm9712: Use core AC'97 reset helper
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm9712.c | 45 ++++++++------------------------------------- 1 file changed, 8 insertions(+), 37 deletions(-)
diff --git a/sound/soc/codecs/wm9712.c b/sound/soc/codecs/wm9712.c index 1fda104dfc45..488a92224249 100644 --- a/sound/soc/codecs/wm9712.c +++ b/sound/soc/codecs/wm9712.c @@ -23,6 +23,9 @@ #include <sound/tlv.h> #include "wm9712.h"
+#define WM9712_VENDOR_ID 0x574d4c12 +#define WM9712_VENDOR_ID_MASK 0xffffffff + struct wm9712_priv { struct snd_ac97 *ac97; unsigned int hp_mixer[2]; @@ -613,35 +616,14 @@ static int wm9712_set_bias_level(struct snd_soc_codec *codec, return 0; }
-static int wm9712_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(wm9712->ac97); - if (ac97_read(codec, 0) == wm9712_reg[0]) - return 1; - } - - soc_ac97_ops->reset(wm9712->ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(wm9712->ac97); - if (ac97_read(codec, 0) != wm9712_reg[0]) - goto err; - return 0; - -err: - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - return -EIO; -} - static int wm9712_soc_resume(struct snd_soc_codec *codec) { struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9712_reset(codec, 1); + ret = snd_ac97_reset(wm9712->ac97, true, WM9712_VENDOR_ID, + WM9712_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -663,31 +645,20 @@ static int wm9712_soc_resume(struct snd_soc_codec *codec) static int wm9712_soc_probe(struct snd_soc_codec *codec) { struct wm9712_priv *wm9712 = snd_soc_codec_get_drvdata(codec); - int ret = 0; + int ret;
- wm9712->ac97 = snd_soc_alloc_ac97_codec(codec); + wm9712->ac97 = snd_soc_new_ac97_codec(codec, WM9712_VENDOR_ID, + WM9712_VENDOR_ID_MASK); if (IS_ERR(wm9712->ac97)) { ret = PTR_ERR(wm9712->ac97); dev_err(codec->dev, "Failed to register AC97 codec: %d\n", ret); return ret; }
- ret = wm9712_reset(codec, 0); - if (ret < 0) - goto err_put_device; - - ret = device_add(&wm9712->ac97->dev); - if (ret) - goto err_put_device; - /* set alc mux to none */ ac97_write(codec, AC97_VIDEO, ac97_read(codec, AC97_VIDEO) | 0x3000);
return 0; - -err_put_device: - put_device(&wm9712->ac97->dev); - return ret; }
static int wm9712_soc_remove(struct snd_soc_codec *codec)
On Tue, Jul 21, 2015 at 09:53:05PM +0200, Lars-Peter Clausen wrote:
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Acked-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Thanks, Charles
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/wm9713.c | 48 ++++++++--------------------------------------- sound/soc/codecs/wm9713.h | 2 -- 2 files changed, 8 insertions(+), 42 deletions(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 89cd2d6..955e651 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -29,6 +29,9 @@
#include "wm9713.h"
+#define WM9713_VENDOR_ID 0x574d4c13 +#define WM9713_VENDOR_ID_MASK 0xffffffff + struct wm9713_priv { struct snd_ac97 *ac97; u32 pll_in; /* PLL input frequency */ @@ -1123,28 +1126,6 @@ static struct snd_soc_dai_driver wm9713_dai[] = { }, };
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(wm9713->ac97); - if (ac97_read(codec, 0) == wm9713_reg[0]) - return 1; - } - - soc_ac97_ops->reset(wm9713->ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(wm9713->ac97); - if (ac97_read(codec, 0) != wm9713_reg[0]) { - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - return -EIO; - } - - return 0; -} -EXPORT_SYMBOL_GPL(wm9713_reset); - static int wm9713_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { @@ -1196,7 +1177,8 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9713_reset(codec, 1); + ret = snd_ac97_reset(wm9713->ac97, true, WM9713_VENDOR_ID, + WM9713_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -1222,32 +1204,18 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) static int wm9713_soc_probe(struct snd_soc_codec *codec) { struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec); - int ret = 0, reg; + int reg;
- wm9713->ac97 = snd_soc_alloc_ac97_codec(codec); + wm9713->ac97 = snd_soc_new_ac97_codec(codec, WM9713_VENDOR_ID, + WM9713_VENDOR_ID_MASK); if (IS_ERR(wm9713->ac97)) return PTR_ERR(wm9713->ac97);
- /* do a cold reset for the controller and then try - * a warm reset followed by an optional cold reset for codec */ - wm9713_reset(codec, 0); - ret = wm9713_reset(codec, 1); - if (ret < 0) - goto err_put_device; - - ret = device_add(&wm9713->ac97->dev); - if (ret) - goto err_put_device; - /* unmute the adc - move to kcontrol */ reg = ac97_read(codec, AC97_CD) & 0x7fff; ac97_write(codec, AC97_CD, reg);
return 0; - -err_put_device: - put_device(&wm9713->ac97->dev); - return ret; }
static int wm9713_soc_remove(struct snd_soc_codec *codec) diff --git a/sound/soc/codecs/wm9713.h b/sound/soc/codecs/wm9713.h index 793da86..53df11b 100644 --- a/sound/soc/codecs/wm9713.h +++ b/sound/soc/codecs/wm9713.h @@ -45,6 +45,4 @@ #define WM9713_DAI_AC97_AUX 1 #define WM9713_DAI_PCM_VOICE 2
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm); - #endif
On Tue, 21 Jul 2015 21:53:06 +0200, Lars-Peter Clausen wrote:
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Better to mention about the removal of the exported wm9713_reset(). I know it's not used anywhere else and safe to remove, but I had to go git-grep for figuring it out.
thanks,
Takashi
sound/soc/codecs/wm9713.c | 48 ++++++++--------------------------------------- sound/soc/codecs/wm9713.h | 2 -- 2 files changed, 8 insertions(+), 42 deletions(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 89cd2d6..955e651 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -29,6 +29,9 @@
#include "wm9713.h"
+#define WM9713_VENDOR_ID 0x574d4c13 +#define WM9713_VENDOR_ID_MASK 0xffffffff
struct wm9713_priv { struct snd_ac97 *ac97; u32 pll_in; /* PLL input frequency */ @@ -1123,28 +1126,6 @@ static struct snd_soc_dai_driver wm9713_dai[] = { }, };
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm) -{
- struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
- if (try_warm && soc_ac97_ops->warm_reset) {
soc_ac97_ops->warm_reset(wm9713->ac97);if (ac97_read(codec, 0) == wm9713_reg[0])return 1;- }
- soc_ac97_ops->reset(wm9713->ac97);
- if (soc_ac97_ops->warm_reset)
soc_ac97_ops->warm_reset(wm9713->ac97);- if (ac97_read(codec, 0) != wm9713_reg[0]) {
dev_err(codec->dev, "Failed to reset: AC97 link error\n");return -EIO;- }
- return 0;
-} -EXPORT_SYMBOL_GPL(wm9713_reset);
static int wm9713_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { @@ -1196,7 +1177,8 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9713_reset(codec, 1);
- ret = snd_ac97_reset(wm9713->ac97, true, WM9713_VENDOR_ID,
WM9713_VENDOR_ID_MASK);@@ -1222,32 +1204,18 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) static int wm9713_soc_probe(struct snd_soc_codec *codec) { struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
- int ret = 0, reg;
- int reg;
- wm9713->ac97 = snd_soc_alloc_ac97_codec(codec);
- wm9713->ac97 = snd_soc_new_ac97_codec(codec, WM9713_VENDOR_ID,
WM9713_VENDOR_ID_MASK);
/* do a cold reset for the controller and then try
* a warm reset followed by an optional cold reset for codec */wm9713_reset(codec, 0);
ret = wm9713_reset(codec, 1);
if (ret < 0)
goto err_put_device;ret = device_add(&wm9713->ac97->dev);
if (ret)
goto err_put_device;/* unmute the adc - move to kcontrol */ reg = ac97_read(codec, AC97_CD) & 0x7fff; ac97_write(codec, AC97_CD, reg);
return 0;
-err_put_device:
- put_device(&wm9713->ac97->dev);
- return ret;
}
static int wm9713_soc_remove(struct snd_soc_codec *codec) diff --git a/sound/soc/codecs/wm9713.h b/sound/soc/codecs/wm9713.h index 793da86..53df11b 100644 --- a/sound/soc/codecs/wm9713.h +++ b/sound/soc/codecs/wm9713.h @@ -45,6 +45,4 @@ #define WM9713_DAI_AC97_AUX 1 #define WM9713_DAI_PCM_VOICE 2
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm);
#endif
2.1.4
The patch
ASoC: wm9713: Use core AC'97 reset helper
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 310398f5e4618e9a0f6fd0c4b152401daf06c215 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen lars@metafoo.de Date: Tue, 21 Jul 2015 21:53:06 +0200 Subject: [PATCH] ASoC: wm9713: Use core AC'97 reset helper
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Reviewed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm9713.c | 48 ++++++++--------------------------------------- sound/soc/codecs/wm9713.h | 2 -- 2 files changed, 8 insertions(+), 42 deletions(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 89cd2d6f57c0..955e6511af56 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -29,6 +29,9 @@
#include "wm9713.h"
+#define WM9713_VENDOR_ID 0x574d4c13 +#define WM9713_VENDOR_ID_MASK 0xffffffff + struct wm9713_priv { struct snd_ac97 *ac97; u32 pll_in; /* PLL input frequency */ @@ -1123,28 +1126,6 @@ static struct snd_soc_dai_driver wm9713_dai[] = { }, };
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm) -{ - struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec); - - if (try_warm && soc_ac97_ops->warm_reset) { - soc_ac97_ops->warm_reset(wm9713->ac97); - if (ac97_read(codec, 0) == wm9713_reg[0]) - return 1; - } - - soc_ac97_ops->reset(wm9713->ac97); - if (soc_ac97_ops->warm_reset) - soc_ac97_ops->warm_reset(wm9713->ac97); - if (ac97_read(codec, 0) != wm9713_reg[0]) { - dev_err(codec->dev, "Failed to reset: AC97 link error\n"); - return -EIO; - } - - return 0; -} -EXPORT_SYMBOL_GPL(wm9713_reset); - static int wm9713_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { @@ -1196,7 +1177,8 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) int i, ret; u16 *cache = codec->reg_cache;
- ret = wm9713_reset(codec, 1); + ret = snd_ac97_reset(wm9713->ac97, true, WM9713_VENDOR_ID, + WM9713_VENDOR_ID_MASK); if (ret < 0) return ret;
@@ -1222,32 +1204,18 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec) static int wm9713_soc_probe(struct snd_soc_codec *codec) { struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec); - int ret = 0, reg; + int reg;
- wm9713->ac97 = snd_soc_alloc_ac97_codec(codec); + wm9713->ac97 = snd_soc_new_ac97_codec(codec, WM9713_VENDOR_ID, + WM9713_VENDOR_ID_MASK); if (IS_ERR(wm9713->ac97)) return PTR_ERR(wm9713->ac97);
- /* do a cold reset for the controller and then try - * a warm reset followed by an optional cold reset for codec */ - wm9713_reset(codec, 0); - ret = wm9713_reset(codec, 1); - if (ret < 0) - goto err_put_device; - - ret = device_add(&wm9713->ac97->dev); - if (ret) - goto err_put_device; - /* unmute the adc - move to kcontrol */ reg = ac97_read(codec, AC97_CD) & 0x7fff; ac97_write(codec, AC97_CD, reg);
return 0; - -err_put_device: - put_device(&wm9713->ac97->dev); - return ret; }
static int wm9713_soc_remove(struct snd_soc_codec *codec) diff --git a/sound/soc/codecs/wm9713.h b/sound/soc/codecs/wm9713.h index 793da863a03d..53df11b1f727 100644 --- a/sound/soc/codecs/wm9713.h +++ b/sound/soc/codecs/wm9713.h @@ -45,6 +45,4 @@ #define WM9713_DAI_AC97_AUX 1 #define WM9713_DAI_PCM_VOICE 2
-int wm9713_reset(struct snd_soc_codec *codec, int try_warm); - #endif
On Tue, Jul 21, 2015 at 09:53:06PM +0200, Lars-Peter Clausen wrote:
Use the new snd_ac97_reset() helper and the reset functionality provided by snd_soc_new_ac97_codec() to perform the device reset rather than open-coding it.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Acked-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Thanks, Charles
On Tue, 21 Jul 2015 21:52:59 +0200, Lars-Peter Clausen wrote:
AC'97 devices require that they are properly reset before they can be used. Currently ASoC drivers do this by hand each on its own. This lead to many similar but slightly different implementations. E.g. Some supporting warm reset, some don't. And even though AC'97 do have a device ID which uniquely identifies a device most drivers only use the content of register 0 for device verification.
This patch series introduces a new generic helper function in the ALSA AC'97 core that can be used to reset a device and verify its ID if required. And then converts drivers to use this new helper function instead of custom code.
snd_soc_new_ac97_codec() is extended to be able to perform a reset between the allocation and registration of the AC'97 device, this will allow more drivers to use it.
- Lars
Lars-Peter Clausen (7): ALSA: ac97: Add helper function to reset the AC97 device ASoC: ac97: Add support for resetting device before registration ASoC: ad1980: Use core AC'97 reset helper ASoC: stac9766: Use core reset helper ASoC: wm9705: Use core AC'97 reset helper ASoC: wm9712: Use core AC'97 reset helper ASoC: wm9713: Use core AC'97 reset helper
Feel free to take my ack for all: Reviewed-by: Takashi Iwai tiwai@suse.de
thanks,
Takashi
participants (4)
-
Charles Keepax -
Lars-Peter Clausen -
Mark Brown -
Takashi Iwai