This set of patches factors out some repeated code to clean up
firmware control read/write functions, and removes some redundant
control notification code.
Simon Trimmer (5):
firmware: cs_dsp: Add locked wrappers for coeff read and write
ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
ALSA: hda: hda_cs_dsp_ctl: Use
cs_dsp_coeff_lock_and_[read|write]_ctrl()
ASoC: wm_adsp: Remove notification of driver write
ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
drivers/firmware/cirrus/cs_dsp.c | 54 ++++++++++++++++++++++++++
include/linux/firmware/cirrus/cs_dsp.h | 4 ++
sound/pci/hda/hda_cs_dsp_ctl.c | 22 +----------
sound/soc/codecs/wm_adsp.c | 32 ++++-----------
4 files changed, 67 insertions(+), 45 deletions(-)
--
2.39.2
From: Simon Trimmer <[email protected]>
Using the cs_dsp_coeff_lock_and_[read|write]_ctrl() wrappers tidies
the calling functions as it does not need to manage the DSP pwr_lock.
Signed-off-by: Simon Trimmer <[email protected]>
Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/soc/codecs/wm_adsp.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 7d5c096e06cd..8e366902ee4e 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -403,13 +403,8 @@ static int wm_coeff_put(struct snd_kcontrol *kctl,
struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
char *p = ucontrol->value.bytes.data;
- int ret = 0;
-
- mutex_lock(&cs_ctl->dsp->pwr_lock);
- ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, p, cs_ctl->len);
- mutex_unlock(&cs_ctl->dsp->pwr_lock);
- return ret;
+ return cs_dsp_coeff_lock_and_write_ctrl(cs_ctl, 0, p, cs_ctl->len);
}
static int wm_coeff_tlv_put(struct snd_kcontrol *kctl,
@@ -426,13 +421,11 @@ static int wm_coeff_tlv_put(struct snd_kcontrol *kctl,
if (!scratch)
return -ENOMEM;
- if (copy_from_user(scratch, bytes, size)) {
+ if (copy_from_user(scratch, bytes, size))
ret = -EFAULT;
- } else {
- mutex_lock(&cs_ctl->dsp->pwr_lock);
- ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, scratch, size);
- mutex_unlock(&cs_ctl->dsp->pwr_lock);
- }
+ else
+ ret = cs_dsp_coeff_lock_and_write_ctrl(cs_ctl, 0, scratch, size);
+
vfree(scratch);
return ret;
@@ -474,13 +467,8 @@ static int wm_coeff_get(struct snd_kcontrol *kctl,
struct wm_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
char *p = ucontrol->value.bytes.data;
- int ret;
- mutex_lock(&cs_ctl->dsp->pwr_lock);
- ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, p, cs_ctl->len);
- mutex_unlock(&cs_ctl->dsp->pwr_lock);
-
- return ret;
+ return cs_dsp_coeff_lock_and_read_ctrl(cs_ctl, 0, p, cs_ctl->len);
}
static int wm_coeff_tlv_get(struct snd_kcontrol *kctl,
--
2.39.2
From: Simon Trimmer <[email protected]>
Any control that the driver is updating should be marked as SYSTEM and
therefore will not have an ALSA control to notify.
Signed-off-by: Simon Trimmer <[email protected]>
Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/soc/codecs/wm_adsp.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 8e366902ee4e..6e348d49a89c 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -672,7 +672,6 @@ int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, int type,
unsigned int alg, void *buf, size_t len)
{
struct cs_dsp_coeff_ctl *cs_ctl;
- struct wm_coeff_ctl *ctl;
int ret;
mutex_lock(&dsp->cs_dsp.pwr_lock);
@@ -683,12 +682,7 @@ int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, int type,
if (ret < 0)
return ret;
- if (ret == 0 || (cs_ctl->flags & WMFW_CTL_FLAG_SYS))
- return 0;
-
- ctl = cs_ctl->priv;
-
- return snd_soc_component_notify_control(dsp->component, ctl->name);
+ return 0;
}
EXPORT_SYMBOL_GPL(wm_adsp_write_ctl);
--
2.39.2
On Mon, 25 Mar 2024 12:31:22 +0100,
Richard Fitzgerald wrote:
>
> This set of patches factors out some repeated code to clean up
> firmware control read/write functions, and removes some redundant
> control notification code.
>
> Simon Trimmer (5):
> firmware: cs_dsp: Add locked wrappers for coeff read and write
> ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
> ALSA: hda: hda_cs_dsp_ctl: Use
> cs_dsp_coeff_lock_and_[read|write]_ctrl()
> ASoC: wm_adsp: Remove notification of driver write
> ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
The patch 4 doesn't look cleanly applicable to my tree.
Should it be applied via Mark's tree?
thanks,
Takashi
On 30/03/2024 08:40, Takashi Iwai wrote:
> On Mon, 25 Mar 2024 12:31:22 +0100,
> Richard Fitzgerald wrote:
>>
>> This set of patches factors out some repeated code to clean up
>> firmware control read/write functions, and removes some redundant
>> control notification code.
>>
>> Simon Trimmer (5):
>> firmware: cs_dsp: Add locked wrappers for coeff read and write
>> ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
>> ALSA: hda: hda_cs_dsp_ctl: Use
>> cs_dsp_coeff_lock_and_[read|write]_ctrl()
>> ASoC: wm_adsp: Remove notification of driver write
>> ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
>
> The patch 4 doesn't look cleanly applicable to my tree.
> Should it be applied via Mark's tree?
>
Yes, it will need to go through Mark's tree.
Mark's for-next has one extra patch to wm_adsp.c that changes
the same function:
f193957b0fbb ("ASoC: wm_adsp: Fix missing mutex_lock in
wm_adsp_write_ctl()")
On Mon, 01 Apr 2024 11:32:49 +0200,
Richard Fitzgerald wrote:
>
> On 30/03/2024 08:40, Takashi Iwai wrote:
> > On Mon, 25 Mar 2024 12:31:22 +0100,
> > Richard Fitzgerald wrote:
> >>
> >> This set of patches factors out some repeated code to clean up
> >> firmware control read/write functions, and removes some redundant
> >> control notification code.
> >>
> >> Simon Trimmer (5):
> >> firmware: cs_dsp: Add locked wrappers for coeff read and write
> >> ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
> >> ALSA: hda: hda_cs_dsp_ctl: Use
> >> cs_dsp_coeff_lock_and_[read|write]_ctrl()
> >> ASoC: wm_adsp: Remove notification of driver write
> >> ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
> >
> > The patch 4 doesn't look cleanly applicable to my tree.
> > Should it be applied via Mark's tree?
> >
>
> Yes, it will need to go through Mark's tree.
> Mark's for-next has one extra patch to wm_adsp.c that changes
> the same function:
>
> f193957b0fbb ("ASoC: wm_adsp: Fix missing mutex_lock in
> wm_adsp_write_ctl()")
OK, then it should go via Mark's tree.
Feel free to take my ack:
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
On Mon, 25 Mar 2024 11:31:22 +0000, Richard Fitzgerald wrote:
> This set of patches factors out some repeated code to clean up
> firmware control read/write functions, and removes some redundant
> control notification code.
>
> Simon Trimmer (5):
> firmware: cs_dsp: Add locked wrappers for coeff read and write
> ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
> ALSA: hda: hda_cs_dsp_ctl: Use
> cs_dsp_coeff_lock_and_[read|write]_ctrl()
> ASoC: wm_adsp: Remove notification of driver write
> ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] firmware: cs_dsp: Add locked wrappers for coeff read and write
commit: 4d0333798ebbfa1683cc3bc056d1b25b8c24344c
[2/5] ASoC: wm_adsp: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
commit: 3802a9969bd365749f5e34928082cff96ed7769b
[3/5] ALSA: hda: hda_cs_dsp_ctl: Use cs_dsp_coeff_lock_and_[read|write]_ctrl()
commit: 62daf3df8a6b1920f7613e478935443a8f449708
[4/5] ASoC: wm_adsp: Remove notification of driver write
commit: e81f5c9f7d06a69dc505fa6ad351df6cc86a6c2d
[5/5] ALSA: hda: hda_cs_dsp_ctl: Remove notification of driver write
commit: d641def12ec929af6c4f9b1b28efcd3e5dff21b4
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