* [PATCH 1/4] ALSA: hda: hda_component: Introduce component parent structure
2024-06-17 15:41 [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Simon Trimmer
@ 2024-06-17 15:41 ` Simon Trimmer
2024-06-17 15:41 ` [PATCH 2/4] ALSA: hda: hda_component: Change codecs to use " Simon Trimmer
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Simon Trimmer @ 2024-06-17 15:41 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, 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 79736c8782a3..9cf94b9fac17 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] 6+ messages in thread* [PATCH 2/4] ALSA: hda: hda_component: Change codecs to use component parent structure
2024-06-17 15:41 [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Simon Trimmer
2024-06-17 15:41 ` [PATCH 1/4] ALSA: hda: hda_component: Introduce component parent structure Simon Trimmer
@ 2024-06-17 15:41 ` Simon Trimmer
2024-06-17 15:41 ` [PATCH 3/4] ALSA: hda: hda_component: Move codec field into the parent Simon Trimmer
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Simon Trimmer @ 2024-06-17 15:41 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, 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 d54d4d60b03e..32c9d95150ba 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 0923e2589f5f..abe415795d90 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 75f7674c66ee..2794acd4c9ab 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] 6+ messages in thread* [PATCH 3/4] ALSA: hda: hda_component: Move codec field into the parent
2024-06-17 15:41 [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Simon Trimmer
2024-06-17 15:41 ` [PATCH 1/4] ALSA: hda: hda_component: Introduce component parent structure Simon Trimmer
2024-06-17 15:41 ` [PATCH 2/4] ALSA: hda: hda_component: Change codecs to use " Simon Trimmer
@ 2024-06-17 15:41 ` Simon Trimmer
2024-06-17 15:41 ` [PATCH 4/4] ALSA: hda: hda_component: Protect shared data with a mutex Simon Trimmer
2024-06-18 9:53 ` [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Simon Trimmer @ 2024-06-17 15:41 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, 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 32c9d95150ba..9c45d5953c80 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 abe415795d90..53dee0286ee1 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 2794acd4c9ab..7fab19f85968 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] 6+ messages in thread* [PATCH 4/4] ALSA: hda: hda_component: Protect shared data with a mutex
2024-06-17 15:41 [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Simon Trimmer
` (2 preceding siblings ...)
2024-06-17 15:41 ` [PATCH 3/4] ALSA: hda: hda_component: Move codec field into the parent Simon Trimmer
@ 2024-06-17 15:41 ` Simon Trimmer
2024-06-18 9:53 ` [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Simon Trimmer @ 2024-06-17 15:41 UTC (permalink / raw)
To: tiwai
Cc: linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu, 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] 6+ messages in thread* Re: [PATCH v3 0/4] ALSA: hda: Improvements to hda_component
2024-06-17 15:41 [PATCH v3 0/4] ALSA: hda: Improvements to hda_component Simon Trimmer
` (3 preceding siblings ...)
2024-06-17 15:41 ` [PATCH 4/4] ALSA: hda: hda_component: Protect shared data with a mutex Simon Trimmer
@ 2024-06-18 9:53 ` Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2024-06-18 9:53 UTC (permalink / raw)
To: Simon Trimmer
Cc: tiwai, linux-sound, alsa-devel, linux-kernel, patches, soyer,
shenghao-ding, kevin-lu, baojun.xu
On Mon, 17 Jun 2024 17:41:01 +0200,
Simon Trimmer wrote:
>
> This series of patches 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.
>
> Changes in v3:
> - These Fixes separated from this series to make them easier
> to manage:
> https://lore.kernel.org/all/20240613133713.75550-1-simont@opensource.cirrus.com/
>
> Simon Trimmer (4):
> 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
Applied all patches now to for-next branch. Thanks.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread