* [PATCH 1/7] ALSA: hda: cs35l56: Component should be unbound before deconstruction
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 2/7] ALSA: hda: cs35l41: " Simon Trimmer
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
The interface associated with the hda_component should be deactivated
before the driver is deconstructed during removal.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/cs35l56_hda.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index 0923e2589f5f..e134ede6c5aa 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -1077,12 +1077,12 @@ void cs35l56_hda_remove(struct device *dev)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
+ component_del(cs35l56->base.dev, &cs35l56_hda_comp_ops);
+
pm_runtime_dont_use_autosuspend(cs35l56->base.dev);
pm_runtime_get_sync(cs35l56->base.dev);
pm_runtime_disable(cs35l56->base.dev);
- component_del(cs35l56->base.dev, &cs35l56_hda_comp_ops);
-
cs_dsp_remove(&cs35l56->cs_dsp);
kfree(cs35l56->system_name);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/7] ALSA: hda: cs35l41: Component should be unbound before deconstruction
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
2024-06-03 10:35 ` [PATCH 1/7] ALSA: hda: cs35l56: Component should be unbound before deconstruction Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 3/7] ALSA: hda/tas2781: " Simon Trimmer
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
The interface associated with the hda_component should be deactivated
before the driver is deconstructed during removal.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/cs35l41_hda.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index d54d4d60b03e..031703f010be 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -2019,6 +2019,8 @@ void cs35l41_hda_remove(struct device *dev)
{
struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
+ component_del(cs35l41->dev, &cs35l41_hda_comp_ops);
+
pm_runtime_get_sync(cs35l41->dev);
pm_runtime_dont_use_autosuspend(cs35l41->dev);
pm_runtime_disable(cs35l41->dev);
@@ -2026,8 +2028,6 @@ void cs35l41_hda_remove(struct device *dev)
if (cs35l41->halo_initialized)
cs35l41_remove_dsp(cs35l41);
- component_del(cs35l41->dev, &cs35l41_hda_comp_ops);
-
acpi_dev_put(cs35l41->dacpi);
pm_runtime_put_noidle(cs35l41->dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/7] ALSA: hda/tas2781: Component should be unbound before deconstruction
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
2024-06-03 10:35 ` [PATCH 1/7] ALSA: hda: cs35l56: Component should be unbound before deconstruction Simon Trimmer
2024-06-03 10:35 ` [PATCH 2/7] ALSA: hda: cs35l41: " Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 4/7] ALSA: hda: hda_component: Introduce component parent structure Simon Trimmer
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
The interface associated with the hda_component should be deactivated
before the driver is deconstructed during removal.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/tas2781_hda_i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 75f7674c66ee..fdee6592c502 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -777,11 +777,11 @@ static void tas2781_hda_remove(struct device *dev)
{
struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+ component_del(tas_hda->dev, &tas2781_hda_comp_ops);
+
pm_runtime_get_sync(tas_hda->dev);
pm_runtime_disable(tas_hda->dev);
- component_del(tas_hda->dev, &tas2781_hda_comp_ops);
-
pm_runtime_put_noidle(tas_hda->dev);
tasdevice_remove(tas_hda->priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/7] ALSA: hda: hda_component: Introduce component parent structure
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
` (2 preceding siblings ...)
2024-06-03 10:35 ` [PATCH 3/7] ALSA: hda/tas2781: " Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 5/7] ALSA: hda: hda_component: Change codecs to use " Simon Trimmer
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
In preparation for moving duplicated members from the hda_component
structure introduce a parent structure that wraps the array of
components. This also allows us to confine the knowledge of the maximum
number of entries to the hda_component files and eliminate passing that
redundant information around and making direct accesses to the array.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/hda_component.c | 65 +++++++++++++++++++----------------
sound/pci/hda/hda_component.h | 42 ++++++++++++++--------
sound/pci/hda/patch_realtek.c | 17 +++++----
3 files changed, 71 insertions(+), 53 deletions(-)
diff --git a/sound/pci/hda/hda_component.c b/sound/pci/hda/hda_component.c
index d02589014a3f..b05a0b87d32a 100644
--- a/sound/pci/hda/hda_component.c
+++ b/sound/pci/hda/hda_component.c
@@ -15,35 +15,39 @@
#include "hda_local.h"
#ifdef CONFIG_ACPI
-void hda_component_acpi_device_notify(struct hda_component *comps, int num_comps,
+void hda_component_acpi_device_notify(struct hda_component_parent *parent,
acpi_handle handle, u32 event, void *data)
{
+ struct hda_component *comp;
int i;
- for (i = 0; i < num_comps; i++) {
- if (comps[i].dev && comps[i].acpi_notify)
- comps[i].acpi_notify(acpi_device_handle(comps[i].adev), event,
- comps[i].dev);
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
+ comp = hda_component_from_index(parent, i);
+ if (comp->dev && comp->acpi_notify)
+ comp->acpi_notify(acpi_device_handle(comp->adev), event, comp->dev);
}
}
EXPORT_SYMBOL_NS_GPL(hda_component_acpi_device_notify, SND_HDA_SCODEC_COMPONENT);
int hda_component_manager_bind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps, int num_comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler, void *data)
{
bool support_notifications = false;
struct acpi_device *adev;
+ struct hda_component *comp;
int ret;
int i;
- adev = comps[0].adev;
+ adev = parent->comps[0].adev;
if (!acpi_device_handle(adev))
return 0;
- for (i = 0; i < num_comps; i++)
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
+ comp = hda_component_from_index(parent, i);
support_notifications = support_notifications ||
- comps[i].acpi_notifications_supported;
+ comp->acpi_notifications_supported;
+ }
if (support_notifications) {
ret = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
@@ -61,13 +65,13 @@ int hda_component_manager_bind_acpi_notifications(struct hda_codec *cdc,
EXPORT_SYMBOL_NS_GPL(hda_component_manager_bind_acpi_notifications, SND_HDA_SCODEC_COMPONENT);
void hda_component_manager_unbind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler)
{
struct acpi_device *adev;
int ret;
- adev = comps[0].adev;
+ adev = parent->comps[0].adev;
if (!acpi_device_handle(adev))
return;
@@ -78,21 +82,25 @@ void hda_component_manager_unbind_acpi_notifications(struct hda_codec *cdc,
EXPORT_SYMBOL_NS_GPL(hda_component_manager_unbind_acpi_notifications, SND_HDA_SCODEC_COMPONENT);
#endif /* ifdef CONFIG_ACPI */
-void hda_component_manager_playback_hook(struct hda_component *comps, int num_comps, int action)
+void hda_component_manager_playback_hook(struct hda_component_parent *parent, int action)
{
+ struct hda_component *comp;
int i;
- for (i = 0; i < num_comps; i++) {
- if (comps[i].dev && comps[i].pre_playback_hook)
- comps[i].pre_playback_hook(comps[i].dev, action);
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
+ comp = hda_component_from_index(parent, i);
+ if (comp->dev && comp->pre_playback_hook)
+ comp->pre_playback_hook(comp->dev, action);
}
- for (i = 0; i < num_comps; i++) {
- if (comps[i].dev && comps[i].playback_hook)
- comps[i].playback_hook(comps[i].dev, action);
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
+ comp = hda_component_from_index(parent, i);
+ if (comp->dev && comp->playback_hook)
+ comp->playback_hook(comp->dev, action);
}
- for (i = 0; i < num_comps; i++) {
- if (comps[i].dev && comps[i].post_playback_hook)
- comps[i].post_playback_hook(comps[i].dev, action);
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
+ comp = hda_component_from_index(parent, i);
+ if (comp->dev && comp->post_playback_hook)
+ comp->post_playback_hook(comp->dev, action);
}
}
EXPORT_SYMBOL_NS_GPL(hda_component_manager_playback_hook, SND_HDA_SCODEC_COMPONENT);
@@ -124,22 +132,21 @@ static int hda_comp_match_dev_name(struct device *dev, void *data)
}
int hda_component_manager_bind(struct hda_codec *cdc,
- struct hda_component *comps, int count)
+ struct hda_component_parent *parent)
{
int i;
- /* Init shared data */
- for (i = 0; i < count; ++i) {
- memset(&comps[i], 0, sizeof(comps[i]));
- comps[i].codec = cdc;
- }
+ /* Init shared and component specific data */
+ memset(parent, 0, sizeof(*parent));
+ for (i = 0; i < ARRAY_SIZE(parent->comps); i++)
+ parent->comps[i].codec = cdc;
- return component_bind_all(hda_codec_dev(cdc), comps);
+ return component_bind_all(hda_codec_dev(cdc), &parent->comps);
}
EXPORT_SYMBOL_NS_GPL(hda_component_manager_bind, SND_HDA_SCODEC_COMPONENT);
int hda_component_manager_init(struct hda_codec *cdc,
- struct hda_component *comps, int count,
+ struct hda_component_parent *parent, int count,
const char *bus, const char *hid,
const char *match_str,
const struct component_master_ops *ops)
diff --git a/sound/pci/hda/hda_component.h b/sound/pci/hda/hda_component.h
index c70b3de68ab2..a016f1b942a2 100644
--- a/sound/pci/hda/hda_component.h
+++ b/sound/pci/hda/hda_component.h
@@ -28,18 +28,21 @@ struct hda_component {
void (*post_playback_hook)(struct device *dev, int action);
};
+struct hda_component_parent {
+ struct hda_component comps[HDA_MAX_COMPONENTS];
+};
+
#ifdef CONFIG_ACPI
-void hda_component_acpi_device_notify(struct hda_component *comps, int num_comps,
+void hda_component_acpi_device_notify(struct hda_component_parent *parent,
acpi_handle handle, u32 event, void *data);
int hda_component_manager_bind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps, int num_comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler, void *data);
void hda_component_manager_unbind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler);
#else
-static inline void hda_component_acpi_device_notify(struct hda_component *comps,
- int num_comps,
+static inline void hda_component_acpi_device_notify(struct hda_component_parent *parent,
acpi_handle handle,
u32 event,
void *data)
@@ -47,8 +50,7 @@ static inline void hda_component_acpi_device_notify(struct hda_component *comps,
}
static inline int hda_component_manager_bind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps,
- int num_comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler,
void *data)
@@ -57,17 +59,16 @@ static inline int hda_component_manager_bind_acpi_notifications(struct hda_codec
}
static inline void hda_component_manager_unbind_acpi_notifications(struct hda_codec *cdc,
- struct hda_component *comps,
+ struct hda_component_parent *parent,
acpi_notify_handler handler)
{
}
#endif /* ifdef CONFIG_ACPI */
-void hda_component_manager_playback_hook(struct hda_component *comps, int num_comps,
- int action);
+void hda_component_manager_playback_hook(struct hda_component_parent *parent, int action);
int hda_component_manager_init(struct hda_codec *cdc,
- struct hda_component *comps, int count,
+ struct hda_component_parent *parent, int count,
const char *bus, const char *hid,
const char *match_str,
const struct component_master_ops *ops);
@@ -75,13 +76,24 @@ int hda_component_manager_init(struct hda_codec *cdc,
void hda_component_manager_free(struct hda_codec *cdc,
const struct component_master_ops *ops);
-int hda_component_manager_bind(struct hda_codec *cdc,
- struct hda_component *comps, int count);
+int hda_component_manager_bind(struct hda_codec *cdc, struct hda_component_parent *parent);
+
+static inline struct hda_component *hda_component_from_index(struct hda_component_parent *parent,
+ int index)
+{
+ if (!parent)
+ return NULL;
+
+ if (index < 0 || index >= ARRAY_SIZE(parent->comps))
+ return NULL;
+
+ return &parent->comps[index];
+}
static inline void hda_component_manager_unbind(struct hda_codec *cdc,
- struct hda_component *comps)
+ struct hda_component_parent *parent)
{
- component_unbind_all(hda_codec_dev(cdc), comps);
+ component_unbind_all(hda_codec_dev(cdc), &parent->comps);
}
#endif /* ifndef __HDA_COMPONENT_H__ */
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index aa76d1c88589..ab9d13e7dcaa 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -131,7 +131,7 @@ struct alc_spec {
u8 alc_mute_keycode_map[1];
/* component binding */
- struct hda_component comps[HDA_MAX_COMPONENTS];
+ struct hda_component_parent comps;
};
/*
@@ -6789,8 +6789,7 @@ static void comp_acpi_device_notify(acpi_handle handle, u32 event, void *data)
codec_info(cdc, "ACPI Notification %d\n", event);
- hda_component_acpi_device_notify(spec->comps, ARRAY_SIZE(spec->comps),
- handle, event, data);
+ hda_component_acpi_device_notify(&spec->comps, handle, event, data);
}
static int comp_bind(struct device *dev)
@@ -6799,12 +6798,12 @@ static int comp_bind(struct device *dev)
struct alc_spec *spec = cdc->spec;
int ret;
- ret = hda_component_manager_bind(cdc, spec->comps, ARRAY_SIZE(spec->comps));
+ ret = hda_component_manager_bind(cdc, &spec->comps);
if (ret)
return ret;
return hda_component_manager_bind_acpi_notifications(cdc,
- spec->comps, ARRAY_SIZE(spec->comps),
+ &spec->comps,
comp_acpi_device_notify, cdc);
}
@@ -6813,8 +6812,8 @@ static void comp_unbind(struct device *dev)
struct hda_codec *cdc = dev_to_hda_codec(dev);
struct alc_spec *spec = cdc->spec;
- hda_component_manager_unbind_acpi_notifications(cdc, spec->comps, comp_acpi_device_notify);
- hda_component_manager_unbind(cdc, spec->comps);
+ hda_component_manager_unbind_acpi_notifications(cdc, &spec->comps, comp_acpi_device_notify);
+ hda_component_manager_unbind(cdc, &spec->comps);
}
static const struct component_master_ops comp_master_ops = {
@@ -6827,7 +6826,7 @@ static void comp_generic_playback_hook(struct hda_pcm_stream *hinfo, struct hda_
{
struct alc_spec *spec = cdc->spec;
- hda_component_manager_playback_hook(spec->comps, ARRAY_SIZE(spec->comps), action);
+ hda_component_manager_playback_hook(&spec->comps, action);
}
static void comp_generic_fixup(struct hda_codec *cdc, int action, const char *bus,
@@ -6838,7 +6837,7 @@ static void comp_generic_fixup(struct hda_codec *cdc, int action, const char *bu
switch (action) {
case HDA_FIXUP_ACT_PRE_PROBE:
- ret = hda_component_manager_init(cdc, spec->comps, count, bus, hid,
+ ret = hda_component_manager_init(cdc, &spec->comps, count, bus, hid,
match_str, &comp_master_ops);
if (ret)
return;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/7] ALSA: hda: hda_component: Change codecs to use component parent structure
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
` (3 preceding siblings ...)
2024-06-03 10:35 ` [PATCH 4/7] ALSA: hda: hda_component: Introduce component parent structure Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 6/7] ALSA: hda: hda_component: Move codec field into the parent Simon Trimmer
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
Change the hda_component binding APIs to pass the hds_component_parent
as the parameter so the array of components can be abstracted.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/cs35l41_hda.c | 42 +++++++++++++++++++--------------
sound/pci/hda/cs35l56_hda.c | 25 +++++++++++---------
sound/pci/hda/hda_component.c | 2 +-
sound/pci/hda/hda_component.h | 2 +-
sound/pci/hda/tas2781_hda_i2c.c | 33 +++++++++++++-------------
5 files changed, 57 insertions(+), 47 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index 031703f010be..ceba4f2c85f1 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -1419,27 +1419,28 @@ static void cs35l41_acpi_device_notify(acpi_handle handle, u32 event, struct dev
static int cs35l41_hda_bind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
unsigned int sleep_flags;
int ret = 0;
- if (!comps || cs35l41->index < 0 || cs35l41->index >= HDA_MAX_COMPONENTS)
+ comp = hda_component_from_index(parent, cs35l41->index);
+ if (!comp)
return -EINVAL;
- comps = &comps[cs35l41->index];
- if (comps->dev)
+ if (comp->dev)
return -EBUSY;
pm_runtime_get_sync(dev);
mutex_lock(&cs35l41->fw_mutex);
- comps->dev = dev;
+ comp->dev = dev;
if (!cs35l41->acpi_subsystem_id)
cs35l41->acpi_subsystem_id = kasprintf(GFP_KERNEL, "%.8x",
- comps->codec->core.subsystem_id);
- cs35l41->codec = comps->codec;
- strscpy(comps->name, dev_name(dev), sizeof(comps->name));
+ comp->codec->core.subsystem_id);
+ cs35l41->codec = comp->codec;
+ strscpy(comp->name, dev_name(dev), sizeof(comp->name));
cs35l41->firmware_type = HDA_CS_DSP_FW_SPK_PROT;
@@ -1454,13 +1455,13 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
ret = cs35l41_create_controls(cs35l41);
- comps->playback_hook = cs35l41_hda_playback_hook;
- comps->pre_playback_hook = cs35l41_hda_pre_playback_hook;
- comps->post_playback_hook = cs35l41_hda_post_playback_hook;
- comps->acpi_notify = cs35l41_acpi_device_notify;
- comps->adev = cs35l41->dacpi;
+ comp->playback_hook = cs35l41_hda_playback_hook;
+ comp->pre_playback_hook = cs35l41_hda_pre_playback_hook;
+ comp->post_playback_hook = cs35l41_hda_post_playback_hook;
+ comp->acpi_notify = cs35l41_acpi_device_notify;
+ comp->adev = cs35l41->dacpi;
- comps->acpi_notifications_supported = cs35l41_dsm_supported(acpi_device_handle(comps->adev),
+ comp->acpi_notifications_supported = cs35l41_dsm_supported(acpi_device_handle(comp->adev),
CS35L41_DSM_GET_MUTE);
cs35l41->mute_override = cs35l41_get_acpi_mute_state(cs35l41,
@@ -1469,7 +1470,7 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
mutex_unlock(&cs35l41->fw_mutex);
sleep_flags = lock_system_sleep();
- if (!device_link_add(&comps->codec->core.dev, cs35l41->dev, DL_FLAG_STATELESS))
+ if (!device_link_add(&comp->codec->core.dev, cs35l41->dev, DL_FLAG_STATELESS))
dev_warn(dev, "Unable to create device link\n");
unlock_system_sleep(sleep_flags);
@@ -1489,14 +1490,19 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
unsigned int sleep_flags;
- if (comps[cs35l41->index].dev == dev) {
- memset(&comps[cs35l41->index], 0, sizeof(*comps));
+ comp = hda_component_from_index(parent, cs35l41->index);
+ if (!comp)
+ return;
+
+ if (comp->dev == dev) {
sleep_flags = lock_system_sleep();
device_link_remove(&cs35l41->codec->core.dev, cs35l41->dev);
unlock_system_sleep(sleep_flags);
+ memset(comp, 0, sizeof(*comp));
}
}
diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index e134ede6c5aa..df4498c77171 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -685,20 +685,21 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
static int cs35l56_hda_bind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
int ret;
- if (!comps || cs35l56->index < 0 || cs35l56->index >= HDA_MAX_COMPONENTS)
+ comp = hda_component_from_index(parent, cs35l56->index);
+ if (!comp)
return -EINVAL;
- comps = &comps[cs35l56->index];
- if (comps->dev)
+ if (comp->dev)
return -EBUSY;
- comps->dev = dev;
- cs35l56->codec = comps->codec;
- strscpy(comps->name, dev_name(dev), sizeof(comps->name));
- comps->playback_hook = cs35l56_hda_playback_hook;
+ comp->dev = dev;
+ cs35l56->codec = comp->codec;
+ strscpy(comp->name, dev_name(dev), sizeof(comp->name));
+ comp->playback_hook = cs35l56_hda_playback_hook;
ret = cs35l56_hda_fw_load(cs35l56);
if (ret)
@@ -720,7 +721,8 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
cs35l56_hda_remove_controls(cs35l56);
@@ -732,8 +734,9 @@ static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *
if (cs35l56->base.fw_patched)
cs_dsp_power_down(&cs35l56->cs_dsp);
- if (comps[cs35l56->index].dev == dev)
- memset(&comps[cs35l56->index], 0, sizeof(*comps));
+ comp = hda_component_from_index(parent, cs35l56->index);
+ if (comp && (comp->dev == dev))
+ memset(comp, 0, sizeof(*comp));
cs35l56->codec = NULL;
diff --git a/sound/pci/hda/hda_component.c b/sound/pci/hda/hda_component.c
index b05a0b87d32a..8c11c8b37799 100644
--- a/sound/pci/hda/hda_component.c
+++ b/sound/pci/hda/hda_component.c
@@ -141,7 +141,7 @@ int hda_component_manager_bind(struct hda_codec *cdc,
for (i = 0; i < ARRAY_SIZE(parent->comps); i++)
parent->comps[i].codec = cdc;
- return component_bind_all(hda_codec_dev(cdc), &parent->comps);
+ return component_bind_all(hda_codec_dev(cdc), parent);
}
EXPORT_SYMBOL_NS_GPL(hda_component_manager_bind, SND_HDA_SCODEC_COMPONENT);
diff --git a/sound/pci/hda/hda_component.h b/sound/pci/hda/hda_component.h
index a016f1b942a2..e547e1f1e674 100644
--- a/sound/pci/hda/hda_component.h
+++ b/sound/pci/hda/hda_component.h
@@ -93,7 +93,7 @@ static inline struct hda_component *hda_component_from_index(struct hda_componen
static inline void hda_component_manager_unbind(struct hda_codec *cdc,
struct hda_component_parent *parent)
{
- component_unbind_all(hda_codec_dev(cdc), &parent->comps);
+ component_unbind_all(hda_codec_dev(cdc), parent);
}
#endif /* ifndef __HDA_COMPONENT_H__ */
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index fdee6592c502..c6c1e8e81972 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -706,20 +706,20 @@ static int tas2781_hda_bind(struct device *dev, struct device *master,
void *master_data)
{
struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
struct hda_codec *codec;
unsigned int subid;
int ret;
- if (!comps || tas_hda->priv->index < 0 ||
- tas_hda->priv->index >= HDA_MAX_COMPONENTS)
+ comp = hda_component_from_index(parent, tas_hda->priv->index);
+ if (!comp)
return -EINVAL;
- comps = &comps[tas_hda->priv->index];
- if (comps->dev)
+ if (comp->dev)
return -EBUSY;
- codec = comps->codec;
+ codec = comp->codec;
subid = codec->core.subsystem_id >> 16;
switch (subid) {
@@ -733,13 +733,13 @@ static int tas2781_hda_bind(struct device *dev, struct device *master,
pm_runtime_get_sync(dev);
- comps->dev = dev;
+ comp->dev = dev;
- strscpy(comps->name, dev_name(dev), sizeof(comps->name));
+ strscpy(comp->name, dev_name(dev), sizeof(comp->name));
ret = tascodec_init(tas_hda->priv, codec, THIS_MODULE, tasdev_fw_ready);
if (!ret)
- comps->playback_hook = tas2781_hda_playback_hook;
+ comp->playback_hook = tas2781_hda_playback_hook;
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
@@ -751,13 +751,14 @@ static void tas2781_hda_unbind(struct device *dev,
struct device *master, void *master_data)
{
struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
- struct hda_component *comps = master_data;
- comps = &comps[tas_hda->priv->index];
-
- if (comps->dev == dev) {
- comps->dev = NULL;
- memset(comps->name, 0, sizeof(comps->name));
- comps->playback_hook = NULL;
+ struct hda_component_parent *parent = master_data;
+ struct hda_component *comp;
+
+ comp = hda_component_from_index(parent, tas_hda->priv->index);
+ if (comp && (comp->dev == dev)) {
+ comp->dev = NULL;
+ memset(comp->name, 0, sizeof(comp->name));
+ comp->playback_hook = NULL;
}
tas2781_hda_remove_controls(tas_hda);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/7] ALSA: hda: hda_component: Move codec field into the parent
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
` (4 preceding siblings ...)
2024-06-03 10:35 ` [PATCH 5/7] ALSA: hda: hda_component: Change codecs to use " Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-03 10:35 ` [PATCH 7/7] ALSA: hda: hda_component: Protect shared data with a mutex Simon Trimmer
2024-06-13 12:06 ` [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Takashi Iwai
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
There is one codec shared across all of the bound HDA components and a
copy is usually stashed in the amp driver so it doesn't need to be in
every hda_component.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/cs35l41_hda.c | 7 ++++---
sound/pci/hda/cs35l56_hda.c | 2 +-
sound/pci/hda/hda_component.c | 5 +----
sound/pci/hda/hda_component.h | 2 +-
sound/pci/hda/tas2781_hda_i2c.c | 2 +-
5 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index ceba4f2c85f1..ee9f83b737de 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -1436,10 +1436,11 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
mutex_lock(&cs35l41->fw_mutex);
comp->dev = dev;
+ cs35l41->codec = parent->codec;
if (!cs35l41->acpi_subsystem_id)
cs35l41->acpi_subsystem_id = kasprintf(GFP_KERNEL, "%.8x",
- comp->codec->core.subsystem_id);
- cs35l41->codec = comp->codec;
+ cs35l41->codec->core.subsystem_id);
+
strscpy(comp->name, dev_name(dev), sizeof(comp->name));
cs35l41->firmware_type = HDA_CS_DSP_FW_SPK_PROT;
@@ -1470,7 +1471,7 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
mutex_unlock(&cs35l41->fw_mutex);
sleep_flags = lock_system_sleep();
- if (!device_link_add(&comp->codec->core.dev, cs35l41->dev, DL_FLAG_STATELESS))
+ if (!device_link_add(&cs35l41->codec->core.dev, cs35l41->dev, DL_FLAG_STATELESS))
dev_warn(dev, "Unable to create device link\n");
unlock_system_sleep(sleep_flags);
diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index df4498c77171..cc4aa90db1d1 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -697,7 +697,7 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
return -EBUSY;
comp->dev = dev;
- cs35l56->codec = comp->codec;
+ cs35l56->codec = parent->codec;
strscpy(comp->name, dev_name(dev), sizeof(comp->name));
comp->playback_hook = cs35l56_hda_playback_hook;
diff --git a/sound/pci/hda/hda_component.c b/sound/pci/hda/hda_component.c
index 8c11c8b37799..1a9950b76866 100644
--- a/sound/pci/hda/hda_component.c
+++ b/sound/pci/hda/hda_component.c
@@ -134,12 +134,9 @@ static int hda_comp_match_dev_name(struct device *dev, void *data)
int hda_component_manager_bind(struct hda_codec *cdc,
struct hda_component_parent *parent)
{
- int i;
-
/* Init shared and component specific data */
memset(parent, 0, sizeof(*parent));
- for (i = 0; i < ARRAY_SIZE(parent->comps); i++)
- parent->comps[i].codec = cdc;
+ parent->codec = cdc;
return component_bind_all(hda_codec_dev(cdc), parent);
}
diff --git a/sound/pci/hda/hda_component.h b/sound/pci/hda/hda_component.h
index e547e1f1e674..dd4dabeae9ee 100644
--- a/sound/pci/hda/hda_component.h
+++ b/sound/pci/hda/hda_component.h
@@ -19,7 +19,6 @@
struct hda_component {
struct device *dev;
char name[HDA_MAX_NAME_SIZE];
- struct hda_codec *codec;
struct acpi_device *adev;
bool acpi_notifications_supported;
void (*acpi_notify)(acpi_handle handle, u32 event, struct device *dev);
@@ -29,6 +28,7 @@ struct hda_component {
};
struct hda_component_parent {
+ struct hda_codec *codec;
struct hda_component comps[HDA_MAX_COMPONENTS];
};
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index c6c1e8e81972..d7af4fd10714 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -719,7 +719,7 @@ static int tas2781_hda_bind(struct device *dev, struct device *master,
if (comp->dev)
return -EBUSY;
- codec = comp->codec;
+ codec = parent->codec;
subid = codec->core.subsystem_id >> 16;
switch (subid) {
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/7] ALSA: hda: hda_component: Protect shared data with a mutex
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
` (5 preceding siblings ...)
2024-06-03 10:35 ` [PATCH 6/7] ALSA: hda: hda_component: Move codec field into the parent Simon Trimmer
@ 2024-06-03 10:35 ` Simon Trimmer
2024-06-13 12:06 ` [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Takashi Iwai
7 siblings, 0 replies; 10+ messages in thread
From: Simon Trimmer @ 2024-06-03 10:35 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang, Simon Trimmer
The hda_component contains information shared from the amp drivers to
the codec that can be altered (for example as the driver unloads). Guard
the update and use of these to prevent use of stale data.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
sound/pci/hda/hda_component.c | 13 ++++++++++++-
sound/pci/hda/hda_component.h | 4 ++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_component.c b/sound/pci/hda/hda_component.c
index 1a9950b76866..7b19cb38b4e0 100644
--- a/sound/pci/hda/hda_component.c
+++ b/sound/pci/hda/hda_component.c
@@ -21,11 +21,13 @@ void hda_component_acpi_device_notify(struct hda_component_parent *parent,
struct hda_component *comp;
int i;
+ mutex_lock(&parent->mutex);
for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
comp = hda_component_from_index(parent, i);
if (comp->dev && comp->acpi_notify)
comp->acpi_notify(acpi_device_handle(comp->adev), event, comp->dev);
}
+ mutex_unlock(&parent->mutex);
}
EXPORT_SYMBOL_NS_GPL(hda_component_acpi_device_notify, SND_HDA_SCODEC_COMPONENT);
@@ -87,6 +89,7 @@ void hda_component_manager_playback_hook(struct hda_component_parent *parent, in
struct hda_component *comp;
int i;
+ mutex_lock(&parent->mutex);
for (i = 0; i < ARRAY_SIZE(parent->comps); i++) {
comp = hda_component_from_index(parent, i);
if (comp->dev && comp->pre_playback_hook)
@@ -102,6 +105,7 @@ void hda_component_manager_playback_hook(struct hda_component_parent *parent, in
if (comp->dev && comp->post_playback_hook)
comp->post_playback_hook(comp->dev, action);
}
+ mutex_unlock(&parent->mutex);
}
EXPORT_SYMBOL_NS_GPL(hda_component_manager_playback_hook, SND_HDA_SCODEC_COMPONENT);
@@ -134,11 +138,18 @@ static int hda_comp_match_dev_name(struct device *dev, void *data)
int hda_component_manager_bind(struct hda_codec *cdc,
struct hda_component_parent *parent)
{
+ int ret;
+
/* Init shared and component specific data */
memset(parent, 0, sizeof(*parent));
+ mutex_init(&parent->mutex);
parent->codec = cdc;
- return component_bind_all(hda_codec_dev(cdc), parent);
+ mutex_lock(&parent->mutex);
+ ret = component_bind_all(hda_codec_dev(cdc), parent);
+ mutex_unlock(&parent->mutex);
+
+ return ret;
}
EXPORT_SYMBOL_NS_GPL(hda_component_manager_bind, SND_HDA_SCODEC_COMPONENT);
diff --git a/sound/pci/hda/hda_component.h b/sound/pci/hda/hda_component.h
index dd4dabeae9ee..9f786608144c 100644
--- a/sound/pci/hda/hda_component.h
+++ b/sound/pci/hda/hda_component.h
@@ -11,6 +11,7 @@
#include <linux/acpi.h>
#include <linux/component.h>
+#include <linux/mutex.h>
#include <sound/hda_codec.h>
#define HDA_MAX_COMPONENTS 4
@@ -28,6 +29,7 @@ struct hda_component {
};
struct hda_component_parent {
+ struct mutex mutex;
struct hda_codec *codec;
struct hda_component comps[HDA_MAX_COMPONENTS];
};
@@ -93,7 +95,9 @@ static inline struct hda_component *hda_component_from_index(struct hda_componen
static inline void hda_component_manager_unbind(struct hda_codec *cdc,
struct hda_component_parent *parent)
{
+ mutex_lock(&parent->mutex);
component_unbind_all(hda_codec_dev(cdc), parent);
+ mutex_unlock(&parent->mutex);
}
#endif /* ifndef __HDA_COMPONENT_H__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/7] ALSA: hda: Improvements to hda_component
2024-06-03 10:35 [PATCH v2 0/7] ALSA: hda: Improvements to hda_component Simon Trimmer
` (6 preceding siblings ...)
2024-06-03 10:35 ` [PATCH 7/7] ALSA: hda: hda_component: Protect shared data with a mutex Simon Trimmer
@ 2024-06-13 12:06 ` Takashi Iwai
7 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-06-13 12:06 UTC (permalink / raw)
To: Simon Trimmer
Cc: tiwai, linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, kailang
On Mon, 03 Jun 2024 12:35:17 +0200,
Simon Trimmer wrote:
>
> This series of patches makes sure that the existing consumers of
> the infrastructure unbind their interface before they begin
> destroying the driver, moves duplicated members from the
> instanced component structures into a new parent structure and
> introduces locking so that consumers of the interface do not use
> stale data.
>
> v2:
> - Correct application of sizeof to pointer flagged by kernel test robot
>
> Simon Trimmer (7):
> ALSA: hda: cs35l56: Component should be unbound before deconstruction
> ALSA: hda: cs35l41: Component should be unbound before deconstruction
> ALSA: hda/tas2781: Component should be unbound before deconstruction
> ALSA: hda: hda_component: Introduce component parent structure
> ALSA: hda: hda_component: Change codecs to use component parent
> structure
> ALSA: hda: hda_component: Move codec field into the parent
> ALSA: hda: hda_component: Protect shared data with a mutex
The first three patches look rather like independent fixes.
Could you split those out and add proper Fixes tags, so that stable
trees can pick up?
The rest are a code refactoring and additional protection of the
mutex.
thanks,
Takashi
> sound/pci/hda/cs35l41_hda.c | 47 ++++++++++++---------
> sound/pci/hda/cs35l56_hda.c | 29 +++++++------
> sound/pci/hda/hda_component.c | 75 ++++++++++++++++++++-------------
> sound/pci/hda/hda_component.h | 48 ++++++++++++++-------
> sound/pci/hda/patch_realtek.c | 17 ++++----
> sound/pci/hda/tas2781_hda_i2c.c | 37 ++++++++--------
> 6 files changed, 147 insertions(+), 106 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread