* [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec
@ 2026-09-07 8:37 Srinivas Kandagatla
2026-09-07 8:37 ` [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI Srinivas Kandagatla
` (10 more replies)
0 siblings, 11 replies; 45+ messages in thread
From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Charles Keepax
Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela,
Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart,
Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches,
linux-sound, devicetree, linux-arm-msm, linux-kernel,
Srinivas Kandagatla
This series extends the SDCA class driver to work on DT / non-ACPI
platforms via a codec-provided static topology, and adds the Qualcomm
WCD9378 (Tambora) codec as the first user driven in SDCA / compute
mode. Tested on the Snapdragon X2 Elite Glymur CRD.
Patches 1-9 extend the SDCA class helpers:
- Kconfig gate to build the class without ACPI enabled.
- PM helpers keyed on a codec-embedded struct sdca_class_drv, so
codec drivers can build their own dev_pm_ops.
- Class SoundWire probe/remove/read_prop exposed as library
functions callable from codec-specific SDW slave drivers.
- hw_ops with hw_init (supplies/reset/SCP init) and
populate_function (DT-supplied SDCA function data) hooks.
- Class function driver registers SDCA_FUNCTION_TYPE_SIMPLE_JACK,
and resolves sound-dai cells by entity index.
- sdca_apply_default_control_classifiers() applies the same
is_volatile / has_reset flags that the DisCo/ACPI parser would.
Note:
None of these are functional changes in sdca core, all of these are
helper functions to allow non-ACPI users to use sdca core.
Patches 10-11 add the WCD9378 SDCA / compute-mode codec:
- dt-bindings for the SoundWire slave in compute mode, using the
qcom,wcd9378c variant compatible alongside the SoundWire
class-ID compatible sdw20217011000.
- Codec driver split across wcd9378-sdw.c (SoundWire slave driver
plumbing) and wcd9378-sdca.c (SDCA topology + hw_ops) with a
shared wcd9378-sdca.h header. A future mobile-mode driver can
plug into the same module (single MODULE_DEVICE_TABLE, single
modalias) by adding its own files and hooking into
wcd9378-sdw.c.
Testing: HPH playback and headset-mic capture confirmed on the
Snapdragon X2 Elite Glymur CRD.
Jack detection is not wired up in this series. GE 35 / MBHC stays
armed by the transcribed init table but is not consumed; the mode
0/1 aliasing in range_ge35_mode_data is a defensive fallback and
has not been tested.
Changes since RFC v1 [1]:
- Compute-mode identified by qcom,wcd9378c variant compatible
instead of the qcom,compute-mode boolean property.
- qcom,micbias2-microvolt dropped; MICB2 fixed at 2.75 V by the
SimpleJack topology.
- Codec split into wcd9378-sdw.c (SDW plumbing) and
wcd9378-sdca.c (SDCA topology); module renamed to
snd-soc-wcd9378 as preparatory for mobile driver.
- Binding is compute-only; qcom,wcd9378 mobile-mode variant will
land with the mobile-mode driver.
- MODULE_IMPORT_NS("SND_SOC_SDCA") added.
- sdca_class.h moved from sound/soc/sdca/ to include/sound/.
- qcom,port-mapping missing now returns -EINVAL via
dev_err_probe().
- init_table sync up with CRD asl and the T14s DSDT; 8 stray writes dropped.
- Six fake IRQ-anchor Controls on PDE 47 dropped; HPH
OCP/CNP/SURGE stay codec-local until an SDCA framework
interface for standalone status-bit IRQs lands.
- .is_volatile dropped from FU 6 CHANNEL_VOLUME (matches MUTE).
- dropped pre post pde callbacks, and all of the mixers and mute are
exposed as mixer controls.
- added sdca_apply_default_control_classifiers, inline with acpi
iparsing.
[1] https://lore.kernel.org/all/amHqO5pMFmKRNYTT@opensource.cirrus.com/
Srinivas Kandagatla (11):
ASoC: SDCA: allow building without ACPI
ASoC: SDCA: export PM helpers keyed on sdca_class_drv
ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library
ASoC: SDCA: add hw_ops with hw_init hook
ASoC: SDCA: add populate_function hw_op for DT function data
ASoC: SDCA: class_function: xlate sound-dai cell by entity index
ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function
driver
ASoC: SDCA: make find_sdca_control_reset() return void
ASoC: SDCA: add sdca_apply_default_control_classifiers() helper
dt-bindings: sound: qcom: add Tambora WCD9378 SDCA codec
ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec
.../bindings/sound/qcom,wcd9378-sdw.yaml | 91 ++
include/sound/sdca.h | 23 +-
include/sound/sdca_class.h | 80 ++
include/sound/sdca_function.h | 2 +
sound/soc/codecs/Kconfig | 11 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/wcd9378-sdca.c | 1010 +++++++++++++++++
sound/soc/codecs/wcd9378-sdca.h | 20 +
sound/soc/codecs/wcd9378-sdw.c | 52 +
sound/soc/sdca/Kconfig | 6 +-
sound/soc/sdca/sdca_class.c | 166 ++-
sound/soc/sdca/sdca_class.h | 35 -
sound/soc/sdca/sdca_class_function.c | 50 +-
sound/soc/sdca/sdca_device.c | 4 +
sound/soc/sdca/sdca_functions.c | 43 +-
15 files changed, 1516 insertions(+), 79 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd9378-sdw.yaml
create mode 100644 include/sound/sdca_class.h
create mode 100644 sound/soc/codecs/wcd9378-sdca.c
create mode 100644 sound/soc/codecs/wcd9378-sdca.h
create mode 100644 sound/soc/codecs/wcd9378-sdw.c
delete mode 100644 sound/soc/sdca/sdca_class.h
--
2.53.0
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 8:54 ` Richard Fitzgerald 2026-09-07 8:37 ` [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv Srinivas Kandagatla ` (9 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla The SDCA class driver is useful on non-ACPI platforms where the topology is supplied statically via sdca_class_hw_ops.get_function_data. Drop 'depends on ACPI' from SND_SOC_SDCA and guard the ACPI-parsing implementations in sdca_device.c and sdca_functions.c with IS_ENABLED(CONFIG_ACPI), providing empty stubs when ACPI is off. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- include/sound/sdca.h | 23 ++++++++++++++++++----- sound/soc/sdca/Kconfig | 6 ++++-- sound/soc/sdca/sdca_device.c | 4 ++++ sound/soc/sdca/sdca_functions.c | 2 ++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/include/sound/sdca.h b/include/sound/sdca.h index 2bdf4e333e04..db2672b3aae2 100644 --- a/include/sound/sdca.h +++ b/include/sound/sdca.h @@ -60,20 +60,16 @@ enum sdca_quirk { SDCA_QUIRKS_SKIP_FUNC_TYPE_PATCHING, }; -#if IS_ENABLED(CONFIG_ACPI) && IS_ENABLED(CONFIG_SND_SOC_SDCA) +#if IS_ENABLED(CONFIG_SND_SOC_SDCA) -void sdca_lookup_functions(struct sdw_slave *slave); void sdca_lookup_swft(struct sdw_slave *slave); -void sdca_lookup_interface_revision(struct sdw_slave *slave); bool sdca_device_quirk_match(struct sdw_slave *slave, enum sdca_quirk quirk); int sdca_dev_register_functions(struct sdw_slave *slave); void sdca_dev_unregister_functions(struct sdw_slave *slave); #else -static inline void sdca_lookup_functions(struct sdw_slave *slave) {} static inline void sdca_lookup_swft(struct sdw_slave *slave) {} -static inline void sdca_lookup_interface_revision(struct sdw_slave *slave) {} static inline bool sdca_device_quirk_match(struct sdw_slave *slave, enum sdca_quirk quirk) { return false; @@ -88,4 +84,21 @@ static inline void sdca_dev_unregister_functions(struct sdw_slave *slave) {} #endif +/* + * Called from the SoundWire bus during peripheral enumeration; gated on + * ACPI to avoid a soundwire_bus <-> snd_soc_sdca module cycle on DT builds + * (where the bodies are stubs anyway). + */ +#if IS_ENABLED(CONFIG_ACPI) && IS_ENABLED(CONFIG_SND_SOC_SDCA) + +void sdca_lookup_functions(struct sdw_slave *slave); +void sdca_lookup_interface_revision(struct sdw_slave *slave); + +#else + +static inline void sdca_lookup_functions(struct sdw_slave *slave) {} +static inline void sdca_lookup_interface_revision(struct sdw_slave *slave) {} + +#endif + #endif diff --git a/sound/soc/sdca/Kconfig b/sound/soc/sdca/Kconfig index 4c0dcb9ff3b9..72c02943c49e 100644 --- a/sound/soc/sdca/Kconfig +++ b/sound/soc/sdca/Kconfig @@ -3,11 +3,13 @@ menu "SoundWire (SDCA)" config SND_SOC_SDCA tristate "SDCA core support" - depends on ACPI select AUXILIARY_BUS help This option enables support for the MIPI SoundWire Device - Class for Audio (SDCA). + Class for Audio (SDCA). The ACPI/DisCo topology parser is + only built when CONFIG_ACPI is enabled; on non-ACPI platforms + (e.g. ARM/DT) codec drivers supply the SDCA function data + via sdca_class_hw_ops.populate_function. config SND_SOC_SDCA_HID bool "SDCA HID support" diff --git a/sound/soc/sdca/sdca_device.c b/sound/soc/sdca/sdca_device.c index 4bcd8d1fdff8..3f302bfa5457 100644 --- a/sound/soc/sdca/sdca_device.c +++ b/sound/soc/sdca/sdca_device.c @@ -15,6 +15,7 @@ #include <sound/sdca.h> #include <sound/sdca_function.h> +#if IS_ENABLED(CONFIG_ACPI) void sdca_lookup_interface_revision(struct sdw_slave *slave) { struct fwnode_handle *fwnode = slave->dev.fwnode; @@ -45,6 +46,9 @@ void sdca_lookup_swft(struct sdw_slave *slave) devm_add_action_or_reset(&slave->dev, devm_acpi_table_put, slave->sdca_data.swft); } +#else +void sdca_lookup_swft(struct sdw_slave *slave) { } +#endif EXPORT_SYMBOL_NS(sdca_lookup_swft, "SND_SOC_SDCA"); static bool sdca_device_quirk_rt712_vb(struct sdw_slave *slave) diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c index e01d91eb3cc8..47dedb62c24c 100644 --- a/sound/soc/sdca/sdca_functions.c +++ b/sound/soc/sdca/sdca_functions.c @@ -25,6 +25,7 @@ */ #define SDCA_PROPERTY_LENGTH 64 +#if IS_ENABLED(CONFIG_ACPI) static int patch_sdca_function_type(u32 interface_revision, u32 *function_type) { /* @@ -199,6 +200,7 @@ void sdca_lookup_functions(struct sdw_slave *slave) acpi_dev_for_each_child(adev, find_sdca_function, &slave->sdca_data); } EXPORT_SYMBOL_NS(sdca_lookup_functions, "SND_SOC_SDCA"); +#endif struct raw_init_write { __le32 addr; -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI 2026-09-07 8:37 ` [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI Srinivas Kandagatla @ 2026-09-07 8:54 ` Richard Fitzgerald 2026-09-07 9:09 ` Takashi Iwai 0 siblings, 1 reply; 45+ messages in thread From: Richard Fitzgerald @ 2026-09-07 8:54 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 07/09/2026 9:37 am, Srinivas Kandagatla wrote: > The SDCA class driver is useful on non-ACPI platforms where the > topology is supplied statically via sdca_class_hw_ops.get_function_data. > > Drop 'depends on ACPI' from SND_SOC_SDCA and guard the > ACPI-parsing implementations in sdca_device.c and sdca_functions.c > with IS_ENABLED(CONFIG_ACPI), providing empty stubs when ACPI is off. Would it be better to use IS_REACHABLE(CONFIG_ACPI)? ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI 2026-09-07 8:54 ` Richard Fitzgerald @ 2026-09-07 9:09 ` Takashi Iwai 0 siblings, 0 replies; 45+ messages in thread From: Takashi Iwai @ 2026-09-07 9:09 UTC (permalink / raw) To: Richard Fitzgerald Cc: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, 07 Sep 2026 10:54:08 +0200, Richard Fitzgerald wrote: > > On 07/09/2026 9:37 am, Srinivas Kandagatla wrote: > > The SDCA class driver is useful on non-ACPI platforms where the > > topology is supplied statically via sdca_class_hw_ops.get_function_data. > > > > Drop 'depends on ACPI' from SND_SOC_SDCA and guard the > > ACPI-parsing implementations in sdca_device.c and sdca_functions.c > > with IS_ENABLED(CONFIG_ACPI), providing empty stubs when ACPI is off. > > Would it be better to use IS_REACHABLE(CONFIG_ACPI)? CONFIG_ACPI is a bool, so it's equivalent. thanks, Takashi ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-08 16:22 ` Charles Keepax 2026-09-07 8:37 ` [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library Srinivas Kandagatla ` (8 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla The class PM callbacks pull sdca_class_drv out of drvdata, so the built-in class_sdw_driver owns the drvdata slot. That works for the generic case but blocks codec drivers that want to embed sdca_class_drv in their own private struct -- they need drvdata for their codec priv. Split the four callbacks into exported helpers that take a struct sdca_class_drv * directly: sdca_class_system_suspend() sdca_class_system_resume() sdca_class_runtime_suspend() sdca_class_runtime_resume() Codec drivers can now compose these into their own dev_pm_ops without going through drvdata. For the built-in class_sdw_driver, add small dev_pm_ops wrappers that fetch drv from drvdata, wire them into sdca_class_pm_ops, and export the ops so any generic SDCA slave can pick them up as-is. No functional change: the built-in class_sdw_driver keeps the same PM semantics; only the internal plumbing shifts to operate on sdca_class_drv instead of struct device *dev. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- .../soc/sdca => include/sound}/sdca_class.h | 19 +++++ sound/soc/sdca/sdca_class.c | 82 +++++++++++++++---- sound/soc/sdca/sdca_class_function.c | 2 +- 3 files changed, 84 insertions(+), 19 deletions(-) rename {sound/soc/sdca => include/sound}/sdca_class.h (50%) diff --git a/sound/soc/sdca/sdca_class.h b/include/sound/sdca_class.h similarity index 50% rename from sound/soc/sdca/sdca_class.h rename to include/sound/sdca_class.h index 57f7f8d08f49..15a180385202 100644 --- a/sound/soc/sdca/sdca_class.h +++ b/include/sound/sdca_class.h @@ -15,6 +15,7 @@ #include <linux/workqueue.h> struct device; +struct dev_pm_ops; struct regmap; struct sdw_slave; struct sdca_function_data; @@ -32,4 +33,22 @@ struct sdca_class_drv { struct work_struct boot_work; }; +/* + * PM helpers. Codec drivers embed sdca_class_drv in their own priv, + * own dev_set_drvdata(), and compose these into their own dev_pm_ops: + * + * static int wcd_runtime_suspend(struct device *dev) { + * struct wcd_priv *priv = dev_get_drvdata(dev); + * return sdca_class_runtime_suspend(&priv->class); + * } + * + * The built-in class_sdw_driver in sdca_class.c uses sdca_class_pm_ops + * directly because it stashes the sdca_class_drv in drvdata itself. + */ +int sdca_class_runtime_suspend(struct sdca_class_drv *drv); +int sdca_class_runtime_resume(struct sdca_class_drv *drv); +int sdca_class_system_suspend(struct sdca_class_drv *drv); +int sdca_class_system_resume(struct sdca_class_drv *drv); +extern const struct dev_pm_ops sdca_class_pm_ops; + #endif /* __SDCA_CLASS_H__ */ diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c index d7444f442c71..88a29116a334 100644 --- a/sound/soc/sdca/sdca_class.c +++ b/sound/soc/sdca/sdca_class.c @@ -20,7 +20,7 @@ #include <sound/sdca_function.h> #include <sound/sdca_interrupts.h> #include <sound/sdca_regmap.h> -#include "sdca_class.h" +#include <sound/sdca_class.h> #define CLASS_SDW_ATTACH_TIMEOUT_MS 5000 @@ -194,30 +194,41 @@ static void class_sdw_remove(struct sdw_slave *sdw) cancel_work_sync(&drv->boot_work); } -static int class_suspend(struct device *dev) +/** + * sdca_class_system_suspend - SDCA class system suspend helper + * @drv: caller-owned sdca_class_drv. + * + * Codec drivers compose this into their own dev_pm_ops. Disables the + * SoundWire interrupt and forces runtime suspend of the underlying + * class regmap. + */ +int sdca_class_system_suspend(struct sdca_class_drv *drv) { - struct sdca_class_drv *drv = dev_get_drvdata(dev); int ret; disable_irq(drv->sdw->irq); - ret = pm_runtime_force_suspend(dev); + ret = pm_runtime_force_suspend(drv->dev); if (ret) { - dev_err(dev, "failed to force suspend: %d\n", ret); + dev_err(drv->dev, "failed to force suspend: %d\n", ret); return ret; } return 0; } +EXPORT_SYMBOL_NS_GPL(sdca_class_system_suspend, "SND_SOC_SDCA_CLASS"); -static int class_resume(struct device *dev) +/** + * sdca_class_system_resume - SDCA class system resume helper + * @drv: caller-owned sdca_class_drv. + */ +int sdca_class_system_resume(struct sdca_class_drv *drv) { - struct sdca_class_drv *drv = dev_get_drvdata(dev); int ret; - ret = pm_runtime_force_resume(dev); + ret = pm_runtime_force_resume(drv->dev); if (ret) { - dev_err(dev, "failed to force resume: %d\n", ret); + dev_err(drv->dev, "failed to force resume: %d\n", ret); return ret; } @@ -225,11 +236,14 @@ static int class_resume(struct device *dev) return 0; } +EXPORT_SYMBOL_NS_GPL(sdca_class_system_resume, "SND_SOC_SDCA_CLASS"); -static int class_runtime_suspend(struct device *dev) +/** + * sdca_class_runtime_suspend - SDCA class runtime suspend helper + * @drv: caller-owned sdca_class_drv. + */ +int sdca_class_runtime_suspend(struct sdca_class_drv *drv) { - struct sdca_class_drv *drv = dev_get_drvdata(dev); - /* * Whilst the driver doesn't power the chip down here, going into runtime * suspend lets the SoundWire bus power down, which means the driver @@ -239,10 +253,14 @@ static int class_runtime_suspend(struct device *dev) return 0; } +EXPORT_SYMBOL_NS_GPL(sdca_class_runtime_suspend, "SND_SOC_SDCA_CLASS"); -static int class_runtime_resume(struct device *dev) +/** + * sdca_class_runtime_resume - SDCA class runtime resume helper + * @drv: caller-owned sdca_class_drv. + */ +int sdca_class_runtime_resume(struct sdca_class_drv *drv) { - struct sdca_class_drv *drv = dev_get_drvdata(dev); int ret; ret = sdw_slave_wait_for_init(drv->sdw, CLASS_SDW_ATTACH_TIMEOUT_MS); @@ -265,11 +283,39 @@ static int class_runtime_resume(struct device *dev) return ret; } +EXPORT_SYMBOL_NS_GPL(sdca_class_runtime_resume, "SND_SOC_SDCA_CLASS"); + +/* + * Convenience dev_pm_ops used by the built-in class_sdw_driver, which + * stashes its sdca_class_drv in drvdata directly. Codec drivers that + * embed sdca_class_drv in their own priv compose their own dev_pm_ops + * using the sdca_class_*_suspend/resume helpers above. + */ +static int class_pm_system_suspend(struct device *dev) +{ + return sdca_class_system_suspend(dev_get_drvdata(dev)); +} + +static int class_pm_system_resume(struct device *dev) +{ + return sdca_class_system_resume(dev_get_drvdata(dev)); +} + +static int class_pm_runtime_suspend(struct device *dev) +{ + return sdca_class_runtime_suspend(dev_get_drvdata(dev)); +} + +static int class_pm_runtime_resume(struct device *dev) +{ + return sdca_class_runtime_resume(dev_get_drvdata(dev)); +} -static const struct dev_pm_ops class_pm_ops = { - SYSTEM_SLEEP_PM_OPS(class_suspend, class_resume) - RUNTIME_PM_OPS(class_runtime_suspend, class_runtime_resume, NULL) +const struct dev_pm_ops sdca_class_pm_ops = { + SYSTEM_SLEEP_PM_OPS(class_pm_system_suspend, class_pm_system_resume) + RUNTIME_PM_OPS(class_pm_runtime_suspend, class_pm_runtime_resume, NULL) }; +EXPORT_SYMBOL_NS_GPL(sdca_class_pm_ops, "SND_SOC_SDCA_CLASS"); static const struct sdw_device_id class_sdw_id[] = { SDW_SLAVE_ENTRY(0x01FA, 0x4245, 0), @@ -282,7 +328,7 @@ MODULE_DEVICE_TABLE(sdw, class_sdw_id); static struct sdw_driver class_sdw_driver = { .driver = { .name = "sdca_class", - .pm = pm_ptr(&class_pm_ops), + .pm = pm_ptr(&sdca_class_pm_ops), }, .probe = class_sdw_probe, diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c index cc7045dc26e6..10a2b031b572 100644 --- a/sound/soc/sdca/sdca_class_function.c +++ b/sound/soc/sdca/sdca_class_function.c @@ -26,7 +26,7 @@ #include <sound/soc-component.h> #include <sound/soc-dai.h> #include <sound/soc.h> -#include "sdca_class.h" +#include <sound/sdca_class.h> #include "sdca_function_device.h" struct class_function_drv { -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv 2026-09-07 8:37 ` [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv Srinivas Kandagatla @ 2026-09-08 16:22 ` Charles Keepax 2026-09-08 17:49 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-08 16:22 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 09:37:16AM +0100, Srinivas Kandagatla wrote: > The class PM callbacks pull sdca_class_drv out of drvdata, so the > built-in class_sdw_driver owns the drvdata slot. That works for the > generic case but blocks codec drivers that want to embed > sdca_class_drv in their own private struct -- they need drvdata for > their codec priv. > > Split the four callbacks into exported helpers that take a > struct sdca_class_drv * directly: > > sdca_class_system_suspend() > sdca_class_system_resume() > sdca_class_runtime_suspend() > sdca_class_runtime_resume() > > Codec drivers can now compose these into their own dev_pm_ops without > going through drvdata. > > For the built-in class_sdw_driver, add small dev_pm_ops wrappers that > fetch drv from drvdata, wire them into sdca_class_pm_ops, and export > the ops so any generic SDCA slave can pick them up as-is. > > No functional change: the built-in class_sdw_driver keeps the same PM > semantics; only the internal plumbing shifts to operate on > sdca_class_drv instead of struct device *dev. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > +/* > + * PM helpers. Codec drivers embed sdca_class_drv in their own priv, > + * own dev_set_drvdata(), and compose these into their own dev_pm_ops: > + * > + * static int wcd_runtime_suspend(struct device *dev) { > + * struct wcd_priv *priv = dev_get_drvdata(dev); > + * return sdca_class_runtime_suspend(&priv->class); > + * } > + * > + * The built-in class_sdw_driver in sdca_class.c uses sdca_class_pm_ops > + * directly because it stashes the sdca_class_drv in drvdata itself. > + */ > +int sdca_class_runtime_suspend(struct sdca_class_drv *drv); > +int sdca_class_runtime_resume(struct sdca_class_drv *drv); > +int sdca_class_system_suspend(struct sdca_class_drv *drv); > +int sdca_class_system_resume(struct sdca_class_drv *drv); > +extern const struct dev_pm_ops sdca_class_pm_ops; I don't think we use sdca_class_pm_ops outside the class driver here, I would leave that internal unless we have a need for it. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv 2026-09-08 16:22 ` Charles Keepax @ 2026-09-08 17:49 ` Srinivas Kandagatla 0 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 17:49 UTC (permalink / raw) To: Charles Keepax Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 5:22 PM, Charles Keepax wrote: > On Mon, Sep 07, 2026 at 09:37:16AM +0100, Srinivas Kandagatla wrote: >> The class PM callbacks pull sdca_class_drv out of drvdata, so the >> built-in class_sdw_driver owns the drvdata slot. That works for the >> generic case but blocks codec drivers that want to embed >> sdca_class_drv in their own private struct -- they need drvdata for >> their codec priv. >> >> Split the four callbacks into exported helpers that take a >> struct sdca_class_drv * directly: >> >> sdca_class_system_suspend() >> sdca_class_system_resume() >> sdca_class_runtime_suspend() >> sdca_class_runtime_resume() >> >> Codec drivers can now compose these into their own dev_pm_ops without >> going through drvdata. >> >> For the built-in class_sdw_driver, add small dev_pm_ops wrappers that >> fetch drv from drvdata, wire them into sdca_class_pm_ops, and export >> the ops so any generic SDCA slave can pick them up as-is. >> >> No functional change: the built-in class_sdw_driver keeps the same PM >> semantics; only the internal plumbing shifts to operate on >> sdca_class_drv instead of struct device *dev. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> >> --- >> +/* >> + * PM helpers. Codec drivers embed sdca_class_drv in their own priv, >> + * own dev_set_drvdata(), and compose these into their own dev_pm_ops: >> + * >> + * static int wcd_runtime_suspend(struct device *dev) { >> + * struct wcd_priv *priv = dev_get_drvdata(dev); >> + * return sdca_class_runtime_suspend(&priv->class); >> + * } >> + * >> + * The built-in class_sdw_driver in sdca_class.c uses sdca_class_pm_ops >> + * directly because it stashes the sdca_class_drv in drvdata itself. >> + */ >> +int sdca_class_runtime_suspend(struct sdca_class_drv *drv); >> +int sdca_class_runtime_resume(struct sdca_class_drv *drv); >> +int sdca_class_system_suspend(struct sdca_class_drv *drv); >> +int sdca_class_system_resume(struct sdca_class_drv *drv); >> +extern const struct dev_pm_ops sdca_class_pm_ops; > > I don't think we use sdca_class_pm_ops outside the class driver > here, I would leave that internal unless we have a need for it. You are right, I will remove this in v3. --srini > > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:31 ` Pierre-Louis Bossart 2026-09-07 8:37 ` [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook Srinivas Kandagatla ` (7 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla Split the internal class_sdw_probe/class_sdw_remove/class_read_prop functions into caller-friendly library helpers: sdca_class_read_prop(sdw) sdca_class_probe(sdw, drv) sdca_class_remove(drv) The class_sdw_probe/class_sdw_remove callbacks of the built-in class_sdw_driver are now thin wrappers that allocate a bare sdca_class_drv, stash it in drvdata, and defer to the exported helpers. The exported sdca_class_probe() takes a caller-owned struct sdca_class_drv * so codec-specific SoundWire drivers can embed the class state in their own priv struct, own dev_set_drvdata() themselves, and avoid a second allocation. No functional change for the built-in driver. This lays the groundwork for codec-specific SDCA SoundWire drivers that want to compose the class-side probe with their own quirks; the next patches add the hw_ops mechanism on top. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- include/sound/sdca_class.h | 5 +++ sound/soc/sdca/sdca_class.c | 75 +++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h index 15a180385202..c6063f22be7a 100644 --- a/include/sound/sdca_class.h +++ b/include/sound/sdca_class.h @@ -33,6 +33,11 @@ struct sdca_class_drv { struct work_struct boot_work; }; +/* Library helpers used by codec-specific SDCA SoundWire drivers. */ +int sdca_class_read_prop(struct sdw_slave *sdw); +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); +void sdca_class_remove(struct sdca_class_drv *drv); + /* * PM helpers. Codec drivers embed sdca_class_drv in their own priv, * own dev_set_drvdata(), and compose these into their own dev_pm_ops: diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c index 88a29116a334..b952fa6eb802 100644 --- a/sound/soc/sdca/sdca_class.c +++ b/sound/soc/sdca/sdca_class.c @@ -24,7 +24,15 @@ #define CLASS_SDW_ATTACH_TIMEOUT_MS 5000 -static int class_read_prop(struct sdw_slave *sdw) +/** + * sdca_class_read_prop - fill SDCA-common SoundWire slave properties + * @sdw: SoundWire slave + * + * Exported so codec-specific SoundWire drivers can invoke the SDCA + * common property setup from their own sdw_slave_ops.read_prop, and + * then apply codec-specific overrides inline. + */ +int sdca_class_read_prop(struct sdw_slave *sdw) { struct sdw_slave_prop *prop = &sdw->prop; @@ -36,9 +44,10 @@ static int class_read_prop(struct sdw_slave *sdw) return 0; } +EXPORT_SYMBOL_NS_GPL(sdca_class_read_prop, "SND_SOC_SDCA_CLASS"); static const struct sdw_slave_ops class_sdw_ops = { - .read_prop = class_read_prop, + .read_prop = sdca_class_read_prop, }; static void class_regmap_lock(void *data) @@ -136,18 +145,31 @@ static void class_boot_work(struct work_struct *work) pm_runtime_put_sync(drv->dev); } -static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) +/** + * sdca_class_probe - SDCA class SoundWire slave probe helper + * @sdw: SoundWire slave + * @drv: caller-allocated sdca_class_drv storage. The caller (a codec + * driver, or the built-in class_sdw_driver in this file) owns the + * allocation and sets its own dev_set_drvdata() -- the framework + * does not touch drvdata. Typically embedded in the codec's own + * priv struct so codec drivers can keep per-slave state. + * + * Codec-specific SoundWire drivers call this from their .probe after + * allocating a struct sdca_class_drv (usually embedded in their own + * priv) and setting drvdata to their priv. The framework fills in the + * sdca_class_drv fields, sets up the class regmap, and queues the + * deferred boot work. + */ +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) { struct device *dev = &sdw->dev; struct regmap_config *dev_config; - struct sdca_class_drv *drv; int ret; sdca_lookup_swft(sdw); - drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); if (!drv) - return -ENOMEM; + return -EINVAL; dev_config = devm_kmemdup(dev, &class_dev_regmap_config, sizeof(*dev_config), GFP_KERNEL); @@ -159,8 +181,6 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id mutex_init(&drv->regmap_lock); mutex_init(&drv->init_lock); - dev_set_drvdata(drv->dev, drv); - INIT_WORK(&drv->boot_work, class_boot_work); dev_config->lock_arg = &drv->regmap_lock; @@ -185,14 +205,47 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id return 0; } +EXPORT_SYMBOL_NS_GPL(sdca_class_probe, "SND_SOC_SDCA_CLASS"); -static void class_sdw_remove(struct sdw_slave *sdw) +static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) { - struct device *dev = &sdw->dev; - struct sdca_class_drv *drv = dev_get_drvdata(dev); + struct sdca_class_drv *drv; + /* + * Pure-generic SDCA parts: no codec priv to embed, so allocate a + * bare sdca_class_drv here and stash it in drvdata for the + * built-in PM ops to fetch. + */ + drv = devm_kzalloc(&sdw->dev, sizeof(*drv), GFP_KERNEL); + if (!drv) + return -ENOMEM; + + dev_set_drvdata(&sdw->dev, drv); + + return sdca_class_probe(sdw, drv); +} + +/** + * sdca_class_remove - SDCA class SoundWire slave remove helper + * @drv: caller-owned sdca_class_drv (the one handed to sdca_class_probe()). + * + * Cancels the deferred boot work so devres can safely free @drv and the + * embedding codec priv without racing class_boot_work. Codec-specific + * SoundWire drivers that call sdca_class_probe() must call this from + * their .remove with the same drv pointer they passed to probe. + */ +void sdca_class_remove(struct sdca_class_drv *drv) +{ cancel_work_sync(&drv->boot_work); } +EXPORT_SYMBOL_NS_GPL(sdca_class_remove, "SND_SOC_SDCA_CLASS"); + +static void class_sdw_remove(struct sdw_slave *sdw) +{ + struct sdca_class_drv *drv = dev_get_drvdata(&sdw->dev); + + sdca_class_remove(drv); +} /** * sdca_class_system_suspend - SDCA class system suspend helper -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library 2026-09-07 8:37 ` [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library Srinivas Kandagatla @ 2026-09-07 11:31 ` Pierre-Louis Bossart 2026-09-07 13:29 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:31 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > Split the internal class_sdw_probe/class_sdw_remove/class_read_prop > functions into caller-friendly library helpers: > > sdca_class_read_prop(sdw) > sdca_class_probe(sdw, drv) > sdca_class_remove(drv) > > The class_sdw_probe/class_sdw_remove callbacks of the built-in > class_sdw_driver are now thin wrappers that allocate a bare > sdca_class_drv, stash it in drvdata, and defer to the exported > helpers. > > The exported sdca_class_probe() takes a caller-owned struct > sdca_class_drv * so codec-specific SoundWire drivers can embed the > class state in their own priv struct, own dev_set_drvdata() themselves, > and avoid a second allocation. No functional change for the built-in > driver. > > This lays the groundwork for codec-specific SDCA SoundWire drivers > that want to compose the class-side probe with their own quirks; the > next patches add the hw_ops mechanism on top. I am not sure I fully understand what you mean by "compose the class-side probe with their own quirks". The existing SDCA mechanism is that there's a top-level SoundWire device, whose driver creates auxiliary subdevices for each function. We currently probe drivers for those function devices based on naming. So how would the 'composing' work then? I guess part of the confusion is also the recurring 'codec specific' term used in several patch, but we are talking about functions which are subsets of a codec, no? maybe use hardware specific. Note that if you need customization to the base class mechanism, there's another option that's currently in use in the other OS that shall not be named. Codec providers are not allowed to change the class drivers, which are owned and distributed as part of the base OS. Codec vendors can however provide extension drivers. The premise was that the sound would be 'acceptable' with class drivers, and the extensions would unlock the secret sauce algorithms for the best audio quality. If we are talking about 'composing class-side probe with quirks', it'd be good to clarify the rationale and objectives... edit: I think the idea is really to use the class function drivers as is, but with the function data provided to and used by the subdevice driver probe. But the hw_init doesn't seem required to me, you already have the .status callback that can be used to perform device-level initializations. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > include/sound/sdca_class.h | 5 +++ > sound/soc/sdca/sdca_class.c | 75 +++++++++++++++++++++++++++++++------ > 2 files changed, 69 insertions(+), 11 deletions(-) > > diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h > index 15a180385202..c6063f22be7a 100644 > --- a/include/sound/sdca_class.h > +++ b/include/sound/sdca_class.h > @@ -33,6 +33,11 @@ struct sdca_class_drv { > struct work_struct boot_work; > }; > > +/* Library helpers used by codec-specific SDCA SoundWire drivers. */ > +int sdca_class_read_prop(struct sdw_slave *sdw); > +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); > +void sdca_class_remove(struct sdca_class_drv *drv); > + > /* > * PM helpers. Codec drivers embed sdca_class_drv in their own priv, > * own dev_set_drvdata(), and compose these into their own dev_pm_ops: > diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c > index 88a29116a334..b952fa6eb802 100644 > --- a/sound/soc/sdca/sdca_class.c > +++ b/sound/soc/sdca/sdca_class.c > @@ -24,7 +24,15 @@ > > #define CLASS_SDW_ATTACH_TIMEOUT_MS 5000 > > -static int class_read_prop(struct sdw_slave *sdw) > +/** > + * sdca_class_read_prop - fill SDCA-common SoundWire slave properties > + * @sdw: SoundWire slave > + * > + * Exported so codec-specific SoundWire drivers can invoke the SDCA > + * common property setup from their own sdw_slave_ops.read_prop, and > + * then apply codec-specific overrides inline. > + */ > +int sdca_class_read_prop(struct sdw_slave *sdw) > { > struct sdw_slave_prop *prop = &sdw->prop; > > @@ -36,9 +44,10 @@ static int class_read_prop(struct sdw_slave *sdw) > > return 0; > } > +EXPORT_SYMBOL_NS_GPL(sdca_class_read_prop, "SND_SOC_SDCA_CLASS"); > > static const struct sdw_slave_ops class_sdw_ops = { > - .read_prop = class_read_prop, > + .read_prop = sdca_class_read_prop, > }; > > static void class_regmap_lock(void *data) > @@ -136,18 +145,31 @@ static void class_boot_work(struct work_struct *work) > pm_runtime_put_sync(drv->dev); > } > > -static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) > +/** > + * sdca_class_probe - SDCA class SoundWire slave probe helper > + * @sdw: SoundWire slave > + * @drv: caller-allocated sdca_class_drv storage. The caller (a codec > + * driver, or the built-in class_sdw_driver in this file) owns the > + * allocation and sets its own dev_set_drvdata() -- the framework > + * does not touch drvdata. Typically embedded in the codec's own > + * priv struct so codec drivers can keep per-slave state. > + * > + * Codec-specific SoundWire drivers call this from their .probe after > + * allocating a struct sdca_class_drv (usually embedded in their own > + * priv) and setting drvdata to their priv. The framework fills in the > + * sdca_class_drv fields, sets up the class regmap, and queues the > + * deferred boot work. > + */ > +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) > { > struct device *dev = &sdw->dev; > struct regmap_config *dev_config; > - struct sdca_class_drv *drv; > int ret; > > sdca_lookup_swft(sdw); > > - drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > if (!drv) > - return -ENOMEM; > + return -EINVAL; > > dev_config = devm_kmemdup(dev, &class_dev_regmap_config, > sizeof(*dev_config), GFP_KERNEL); > @@ -159,8 +181,6 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id > mutex_init(&drv->regmap_lock); > mutex_init(&drv->init_lock); > > - dev_set_drvdata(drv->dev, drv); > - > INIT_WORK(&drv->boot_work, class_boot_work); > > dev_config->lock_arg = &drv->regmap_lock; > @@ -185,14 +205,47 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id > > return 0; > } > +EXPORT_SYMBOL_NS_GPL(sdca_class_probe, "SND_SOC_SDCA_CLASS"); > > -static void class_sdw_remove(struct sdw_slave *sdw) > +static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) > { > - struct device *dev = &sdw->dev; > - struct sdca_class_drv *drv = dev_get_drvdata(dev); > + struct sdca_class_drv *drv; > > + /* > + * Pure-generic SDCA parts: no codec priv to embed, so allocate a > + * bare sdca_class_drv here and stash it in drvdata for the > + * built-in PM ops to fetch. > + */ > + drv = devm_kzalloc(&sdw->dev, sizeof(*drv), GFP_KERNEL); > + if (!drv) > + return -ENOMEM; > + > + dev_set_drvdata(&sdw->dev, drv); > + > + return sdca_class_probe(sdw, drv); > +} > + > +/** > + * sdca_class_remove - SDCA class SoundWire slave remove helper > + * @drv: caller-owned sdca_class_drv (the one handed to sdca_class_probe()). > + * > + * Cancels the deferred boot work so devres can safely free @drv and the > + * embedding codec priv without racing class_boot_work. Codec-specific > + * SoundWire drivers that call sdca_class_probe() must call this from > + * their .remove with the same drv pointer they passed to probe. > + */ > +void sdca_class_remove(struct sdca_class_drv *drv) > +{ > cancel_work_sync(&drv->boot_work); > } > +EXPORT_SYMBOL_NS_GPL(sdca_class_remove, "SND_SOC_SDCA_CLASS"); > + > +static void class_sdw_remove(struct sdw_slave *sdw) > +{ > + struct sdca_class_drv *drv = dev_get_drvdata(&sdw->dev); > + > + sdca_class_remove(drv); > +} > > /** > * sdca_class_system_suspend - SDCA class system suspend helper ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library 2026-09-07 11:31 ` Pierre-Louis Bossart @ 2026-09-07 13:29 ` Srinivas Kandagatla 0 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 13:29 UTC (permalink / raw) To: Pierre-Louis Bossart, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 12:31 PM, Pierre-Louis Bossart wrote: > On 9/7/26 10:37, Srinivas Kandagatla wrote: >> Split the internal class_sdw_probe/class_sdw_remove/class_read_prop >> functions into caller-friendly library helpers: >> >> sdca_class_read_prop(sdw) >> sdca_class_probe(sdw, drv) >> sdca_class_remove(drv) >> >> The class_sdw_probe/class_sdw_remove callbacks of the built-in >> class_sdw_driver are now thin wrappers that allocate a bare >> sdca_class_drv, stash it in drvdata, and defer to the exported >> helpers. >> >> The exported sdca_class_probe() takes a caller-owned struct >> sdca_class_drv * so codec-specific SoundWire drivers can embed the >> class state in their own priv struct, own dev_set_drvdata() themselves, >> and avoid a second allocation. No functional change for the built-in >> driver. >> >> This lays the groundwork for codec-specific SDCA SoundWire drivers >> that want to compose the class-side probe with their own quirks; the >> next patches add the hw_ops mechanism on top. > > I am not sure I fully understand what you mean by "compose the > class-side probe with their own quirks". > I meant having their own probe and callback functions to add platform specific quirks if any, before sending it to sdca core. > The existing SDCA mechanism is that there's a top-level SoundWire > device, whose driver creates auxiliary subdevices for each function. We > currently probe drivers for those function devices based on naming. So > how would the 'composing' work then? > > I guess part of the confusion is also the recurring 'codec specific' I agree, I will try to be bit more clear on the comments in next version. > term used in several patch, but we are talking about functions which are > subsets of a codec, no? maybe use hardware specific. > those function drivers will never be touched, all the hardware specific bits will be in the respective codec drivers. > Note that if you need customization to the base class mechanism, there's > another option that's currently in use in the other OS that shall not be > named. Codec providers are not allowed to change the class drivers, > which are owned and distributed as part of the base OS. Codec vendors > can however provide extension drivers. The premise was that the sound > would be 'acceptable' with class drivers, and the extensions would > unlock the secret sauce algorithms for the best audio quality. If we are > talking about 'composing class-side probe with quirks', it'd be good to > clarify the rationale and objectives... The intention is to use generic class drivers as it is and not modify it, which is what the the whole series is attempting to do tbh. There are no such quirks as of today atleast with two Qualcomm SDCA codecs that I have tried (simple Jack and simple amp). But infuture if we require either quirks or some vendor hooks these could be easily handled within the callback hooks. > > edit: I think the idea is really to use the class function drivers as > is, but with the function data provided to and used by the subdevice > driver probe. But the hw_init doesn't seem required to me, you already > have the .status callback that can be used to perform device-level > initializations. Now that all the callback functions from core are gone except populate on, only static table is passed to the core. --srini > > > >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> >> --- >> include/sound/sdca_class.h | 5 +++ >> sound/soc/sdca/sdca_class.c | 75 +++++++++++++++++++++++++++++++------ >> 2 files changed, 69 insertions(+), 11 deletions(-) >> >> diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h >> index 15a180385202..c6063f22be7a 100644 >> --- a/include/sound/sdca_class.h >> +++ b/include/sound/sdca_class.h >> @@ -33,6 +33,11 @@ struct sdca_class_drv { >> struct work_struct boot_work; >> }; >> >> +/* Library helpers used by codec-specific SDCA SoundWire drivers. */ >> +int sdca_class_read_prop(struct sdw_slave *sdw); >> +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); >> +void sdca_class_remove(struct sdca_class_drv *drv); >> + >> /* >> * PM helpers. Codec drivers embed sdca_class_drv in their own priv, >> * own dev_set_drvdata(), and compose these into their own dev_pm_ops: >> diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c >> index 88a29116a334..b952fa6eb802 100644 >> --- a/sound/soc/sdca/sdca_class.c >> +++ b/sound/soc/sdca/sdca_class.c >> @@ -24,7 +24,15 @@ >> >> #define CLASS_SDW_ATTACH_TIMEOUT_MS 5000 >> >> -static int class_read_prop(struct sdw_slave *sdw) >> +/** >> + * sdca_class_read_prop - fill SDCA-common SoundWire slave properties >> + * @sdw: SoundWire slave >> + * >> + * Exported so codec-specific SoundWire drivers can invoke the SDCA >> + * common property setup from their own sdw_slave_ops.read_prop, and >> + * then apply codec-specific overrides inline. >> + */ >> +int sdca_class_read_prop(struct sdw_slave *sdw) >> { >> struct sdw_slave_prop *prop = &sdw->prop; >> >> @@ -36,9 +44,10 @@ static int class_read_prop(struct sdw_slave *sdw) >> >> return 0; >> } >> +EXPORT_SYMBOL_NS_GPL(sdca_class_read_prop, "SND_SOC_SDCA_CLASS"); >> >> static const struct sdw_slave_ops class_sdw_ops = { >> - .read_prop = class_read_prop, >> + .read_prop = sdca_class_read_prop, >> }; >> >> static void class_regmap_lock(void *data) >> @@ -136,18 +145,31 @@ static void class_boot_work(struct work_struct *work) >> pm_runtime_put_sync(drv->dev); >> } >> >> -static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) >> +/** >> + * sdca_class_probe - SDCA class SoundWire slave probe helper >> + * @sdw: SoundWire slave >> + * @drv: caller-allocated sdca_class_drv storage. The caller (a codec >> + * driver, or the built-in class_sdw_driver in this file) owns the >> + * allocation and sets its own dev_set_drvdata() -- the framework >> + * does not touch drvdata. Typically embedded in the codec's own >> + * priv struct so codec drivers can keep per-slave state. >> + * >> + * Codec-specific SoundWire drivers call this from their .probe after >> + * allocating a struct sdca_class_drv (usually embedded in their own >> + * priv) and setting drvdata to their priv. The framework fills in the >> + * sdca_class_drv fields, sets up the class regmap, and queues the >> + * deferred boot work. >> + */ >> +int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) >> { >> struct device *dev = &sdw->dev; >> struct regmap_config *dev_config; >> - struct sdca_class_drv *drv; >> int ret; >> >> sdca_lookup_swft(sdw); >> >> - drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); >> if (!drv) >> - return -ENOMEM; >> + return -EINVAL; >> >> dev_config = devm_kmemdup(dev, &class_dev_regmap_config, >> sizeof(*dev_config), GFP_KERNEL); >> @@ -159,8 +181,6 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id >> mutex_init(&drv->regmap_lock); >> mutex_init(&drv->init_lock); >> >> - dev_set_drvdata(drv->dev, drv); >> - >> INIT_WORK(&drv->boot_work, class_boot_work); >> >> dev_config->lock_arg = &drv->regmap_lock; >> @@ -185,14 +205,47 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id >> >> return 0; >> } >> +EXPORT_SYMBOL_NS_GPL(sdca_class_probe, "SND_SOC_SDCA_CLASS"); >> >> -static void class_sdw_remove(struct sdw_slave *sdw) >> +static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id) >> { >> - struct device *dev = &sdw->dev; >> - struct sdca_class_drv *drv = dev_get_drvdata(dev); >> + struct sdca_class_drv *drv; >> >> + /* >> + * Pure-generic SDCA parts: no codec priv to embed, so allocate a >> + * bare sdca_class_drv here and stash it in drvdata for the >> + * built-in PM ops to fetch. >> + */ >> + drv = devm_kzalloc(&sdw->dev, sizeof(*drv), GFP_KERNEL); >> + if (!drv) >> + return -ENOMEM; >> + >> + dev_set_drvdata(&sdw->dev, drv); >> + >> + return sdca_class_probe(sdw, drv); >> +} >> + >> +/** >> + * sdca_class_remove - SDCA class SoundWire slave remove helper >> + * @drv: caller-owned sdca_class_drv (the one handed to sdca_class_probe()). >> + * >> + * Cancels the deferred boot work so devres can safely free @drv and the >> + * embedding codec priv without racing class_boot_work. Codec-specific >> + * SoundWire drivers that call sdca_class_probe() must call this from >> + * their .remove with the same drv pointer they passed to probe. >> + */ >> +void sdca_class_remove(struct sdca_class_drv *drv) >> +{ >> cancel_work_sync(&drv->boot_work); >> } >> +EXPORT_SYMBOL_NS_GPL(sdca_class_remove, "SND_SOC_SDCA_CLASS"); >> + >> +static void class_sdw_remove(struct sdw_slave *sdw) >> +{ >> + struct sdca_class_drv *drv = dev_get_drvdata(&sdw->dev); >> + >> + sdca_class_remove(drv); >> +} >> >> /** >> * sdca_class_system_suspend - SDCA class system suspend helper > ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (2 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:29 ` Pierre-Louis Bossart 2026-09-07 8:37 ` [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data Srinivas Kandagatla ` (6 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla Add struct sdca_class_hw_ops with a hw_init callback that runs from sdca_class_probe() before the class regmap is created. Codec drivers use it to enable supplies, toggle reset GPIOs and program initial vendor register state. sdca_class_probe() gains an optional const struct sdca_class_hw_ops * argument (NULL for pure-generic SDCA parts) and stashes it on sdca_class_drv for later use. No functional change for the built-in class_sdw_driver, which passes NULL. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- include/sound/sdca_class.h | 17 ++++++++++++++++- sound/soc/sdca/sdca_class.c | 15 +++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h index c6063f22be7a..3342937d09fd 100644 --- a/include/sound/sdca_class.h +++ b/include/sound/sdca_class.h @@ -20,6 +20,17 @@ struct regmap; struct sdw_slave; struct sdca_function_data; +/** + * struct sdca_class_hw_ops - optional codec hardware callbacks + * @hw_init: enable supplies, toggle reset, etc. Runs from sdca_class_probe() + * before the class regmap is created and before the slave is + * ATTACHED; callers needing bus I/O must sdw_slave_wait_for_init() + * first. + */ +struct sdca_class_hw_ops { + int (*hw_init)(struct sdw_slave *slave); +}; + struct sdca_class_drv { struct device *dev; struct regmap *dev_regmap; @@ -27,6 +38,8 @@ struct sdca_class_drv { struct sdca_interrupt_info *irq_info; + const struct sdca_class_hw_ops *hw_ops; + struct mutex regmap_lock; /* Serialise function initialisations */ struct mutex init_lock; @@ -35,7 +48,9 @@ struct sdca_class_drv { /* Library helpers used by codec-specific SDCA SoundWire drivers. */ int sdca_class_read_prop(struct sdw_slave *sdw); -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); +int sdca_class_probe(struct sdw_slave *sdw, + struct sdca_class_drv *drv, + const struct sdca_class_hw_ops *hw_ops); void sdca_class_remove(struct sdca_class_drv *drv); /* diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c index b952fa6eb802..374de7d9e0b5 100644 --- a/sound/soc/sdca/sdca_class.c +++ b/sound/soc/sdca/sdca_class.c @@ -153,6 +153,8 @@ static void class_boot_work(struct work_struct *work) * allocation and sets its own dev_set_drvdata() -- the framework * does not touch drvdata. Typically embedded in the codec's own * priv struct so codec drivers can keep per-slave state. + * @hw_ops: optional device-specific hw_ops (may be NULL for pure-generic + * SDCA parts that need no quirks) * * Codec-specific SoundWire drivers call this from their .probe after * allocating a struct sdca_class_drv (usually embedded in their own @@ -160,7 +162,9 @@ static void class_boot_work(struct work_struct *work) * sdca_class_drv fields, sets up the class regmap, and queues the * deferred boot work. */ -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) +int sdca_class_probe(struct sdw_slave *sdw, + struct sdca_class_drv *drv, + const struct sdca_class_hw_ops *hw_ops) { struct device *dev = &sdw->dev; struct regmap_config *dev_config; @@ -178,9 +182,16 @@ int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) drv->dev = dev; drv->sdw = sdw; + drv->hw_ops = hw_ops; mutex_init(&drv->regmap_lock); mutex_init(&drv->init_lock); + if (hw_ops && hw_ops->hw_init) { + ret = hw_ops->hw_init(sdw); + if (ret) + return dev_err_probe(dev, ret, "hw_init failed\n"); + } + INIT_WORK(&drv->boot_work, class_boot_work); dev_config->lock_arg = &drv->regmap_lock; @@ -222,7 +233,7 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id dev_set_drvdata(&sdw->dev, drv); - return sdca_class_probe(sdw, drv); + return sdca_class_probe(sdw, drv, NULL); } /** -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook 2026-09-07 8:37 ` [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook Srinivas Kandagatla @ 2026-09-07 11:29 ` Pierre-Louis Bossart 2026-09-07 13:33 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:29 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > Add struct sdca_class_hw_ops with a hw_init callback that runs from > sdca_class_probe() before the class regmap is created. Codec drivers > use it to enable supplies, toggle reset GPIOs and program initial > vendor register state. It'd be good to clarify when this hw_init() is supposed to run. Probe and hardware being available are usually two different things. I think this relies on a behavior at the device level where the function subdevices are only created after SoundWire device enumeration. Also the 'hw_init' naming could be confusing, this is used in many codec drivers to track if the hardware has previously been initialized. > sdca_class_probe() gains an optional const struct sdca_class_hw_ops * > argument (NULL for pure-generic SDCA parts) and stashes it on > sdca_class_drv for later use. > > No functional change for the built-in class_sdw_driver, which passes > NULL. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > include/sound/sdca_class.h | 17 ++++++++++++++++- > sound/soc/sdca/sdca_class.c | 15 +++++++++++++-- > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h > index c6063f22be7a..3342937d09fd 100644 > --- a/include/sound/sdca_class.h > +++ b/include/sound/sdca_class.h > @@ -20,6 +20,17 @@ struct regmap; > struct sdw_slave; > struct sdca_function_data; > > +/** > + * struct sdca_class_hw_ops - optional codec hardware callbacks > + * @hw_init: enable supplies, toggle reset, etc. Runs from sdca_class_probe() > + * before the class regmap is created and before the slave is > + * ATTACHED; callers needing bus I/O must sdw_slave_wait_for_init() > + * first. > + */ > +struct sdca_class_hw_ops { > + int (*hw_init)(struct sdw_slave *slave); > +}; > + > struct sdca_class_drv { > struct device *dev; > struct regmap *dev_regmap; > @@ -27,6 +38,8 @@ struct sdca_class_drv { > > struct sdca_interrupt_info *irq_info; > > + const struct sdca_class_hw_ops *hw_ops; > + > struct mutex regmap_lock; > /* Serialise function initialisations */ > struct mutex init_lock; > @@ -35,7 +48,9 @@ struct sdca_class_drv { > > /* Library helpers used by codec-specific SDCA SoundWire drivers. */ > int sdca_class_read_prop(struct sdw_slave *sdw); > -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); > +int sdca_class_probe(struct sdw_slave *sdw, > + struct sdca_class_drv *drv, > + const struct sdca_class_hw_ops *hw_ops); > void sdca_class_remove(struct sdca_class_drv *drv); > > /* > diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c > index b952fa6eb802..374de7d9e0b5 100644 > --- a/sound/soc/sdca/sdca_class.c > +++ b/sound/soc/sdca/sdca_class.c > @@ -153,6 +153,8 @@ static void class_boot_work(struct work_struct *work) > * allocation and sets its own dev_set_drvdata() -- the framework > * does not touch drvdata. Typically embedded in the codec's own > * priv struct so codec drivers can keep per-slave state. > + * @hw_ops: optional device-specific hw_ops (may be NULL for pure-generic > + * SDCA parts that need no quirks) > * > * Codec-specific SoundWire drivers call this from their .probe after > * allocating a struct sdca_class_drv (usually embedded in their own > @@ -160,7 +162,9 @@ static void class_boot_work(struct work_struct *work) > * sdca_class_drv fields, sets up the class regmap, and queues the > * deferred boot work. > */ > -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) > +int sdca_class_probe(struct sdw_slave *sdw, > + struct sdca_class_drv *drv, > + const struct sdca_class_hw_ops *hw_ops) > { > struct device *dev = &sdw->dev; > struct regmap_config *dev_config; > @@ -178,9 +182,16 @@ int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) > > drv->dev = dev; > drv->sdw = sdw; > + drv->hw_ops = hw_ops; > mutex_init(&drv->regmap_lock); > mutex_init(&drv->init_lock); > > + if (hw_ops && hw_ops->hw_init) { > + ret = hw_ops->hw_init(sdw); > + if (ret) > + return dev_err_probe(dev, ret, "hw_init failed\n"); > + } > + nit-pick: should the INIT_WORK be moved higher before this hw_init()? It has nothing to do with regmap and we'd lose the requirement that hw_init() be run before regmap inits. > INIT_WORK(&drv->boot_work, class_boot_work); > > dev_config->lock_arg = &drv->regmap_lock; > @@ -222,7 +233,7 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id > > dev_set_drvdata(&sdw->dev, drv); > > - return sdca_class_probe(sdw, drv); > + return sdca_class_probe(sdw, drv, NULL); > } > > /** ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook 2026-09-07 11:29 ` Pierre-Louis Bossart @ 2026-09-07 13:33 ` Srinivas Kandagatla 0 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 13:33 UTC (permalink / raw) To: Pierre-Louis Bossart, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 12:29 PM, Pierre-Louis Bossart wrote: > On 9/7/26 10:37, Srinivas Kandagatla wrote: >> Add struct sdca_class_hw_ops with a hw_init callback that runs from >> sdca_class_probe() before the class regmap is created. Codec drivers >> use it to enable supplies, toggle reset GPIOs and program initial >> vendor register state. > > It'd be good to clarify when this hw_init() is supposed to run. Probe > and hardware being available are usually two different things. I think > this relies on a behavior at the device level where the function > subdevices are only created after SoundWire device enumeration. This callback is now removed in v3 and using probe to do most of it. --srini > > Also the 'hw_init' naming could be confusing, this is used in many codec > drivers to track if the hardware has previously been initialized. > >> sdca_class_probe() gains an optional const struct sdca_class_hw_ops * >> argument (NULL for pure-generic SDCA parts) and stashes it on >> sdca_class_drv for later use. >> >> No functional change for the built-in class_sdw_driver, which passes >> NULL. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> >> --- >> include/sound/sdca_class.h | 17 ++++++++++++++++- >> sound/soc/sdca/sdca_class.c | 15 +++++++++++++-- >> 2 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h >> index c6063f22be7a..3342937d09fd 100644 >> --- a/include/sound/sdca_class.h >> +++ b/include/sound/sdca_class.h >> @@ -20,6 +20,17 @@ struct regmap; >> struct sdw_slave; >> struct sdca_function_data; >> >> +/** >> + * struct sdca_class_hw_ops - optional codec hardware callbacks >> + * @hw_init: enable supplies, toggle reset, etc. Runs from sdca_class_probe() >> + * before the class regmap is created and before the slave is >> + * ATTACHED; callers needing bus I/O must sdw_slave_wait_for_init() >> + * first. >> + */ >> +struct sdca_class_hw_ops { >> + int (*hw_init)(struct sdw_slave *slave); >> +}; >> + >> struct sdca_class_drv { >> struct device *dev; >> struct regmap *dev_regmap; >> @@ -27,6 +38,8 @@ struct sdca_class_drv { >> >> struct sdca_interrupt_info *irq_info; >> >> + const struct sdca_class_hw_ops *hw_ops; >> + >> struct mutex regmap_lock; >> /* Serialise function initialisations */ >> struct mutex init_lock; >> @@ -35,7 +48,9 @@ struct sdca_class_drv { >> >> /* Library helpers used by codec-specific SDCA SoundWire drivers. */ >> int sdca_class_read_prop(struct sdw_slave *sdw); >> -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv); >> +int sdca_class_probe(struct sdw_slave *sdw, >> + struct sdca_class_drv *drv, >> + const struct sdca_class_hw_ops *hw_ops); >> void sdca_class_remove(struct sdca_class_drv *drv); >> >> /* >> diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c >> index b952fa6eb802..374de7d9e0b5 100644 >> --- a/sound/soc/sdca/sdca_class.c >> +++ b/sound/soc/sdca/sdca_class.c >> @@ -153,6 +153,8 @@ static void class_boot_work(struct work_struct *work) >> * allocation and sets its own dev_set_drvdata() -- the framework >> * does not touch drvdata. Typically embedded in the codec's own >> * priv struct so codec drivers can keep per-slave state. >> + * @hw_ops: optional device-specific hw_ops (may be NULL for pure-generic >> + * SDCA parts that need no quirks) >> * >> * Codec-specific SoundWire drivers call this from their .probe after >> * allocating a struct sdca_class_drv (usually embedded in their own >> @@ -160,7 +162,9 @@ static void class_boot_work(struct work_struct *work) >> * sdca_class_drv fields, sets up the class regmap, and queues the >> * deferred boot work. >> */ >> -int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) >> +int sdca_class_probe(struct sdw_slave *sdw, >> + struct sdca_class_drv *drv, >> + const struct sdca_class_hw_ops *hw_ops) >> { >> struct device *dev = &sdw->dev; >> struct regmap_config *dev_config; >> @@ -178,9 +182,16 @@ int sdca_class_probe(struct sdw_slave *sdw, struct sdca_class_drv *drv) >> >> drv->dev = dev; >> drv->sdw = sdw; >> + drv->hw_ops = hw_ops; >> mutex_init(&drv->regmap_lock); >> mutex_init(&drv->init_lock); >> >> + if (hw_ops && hw_ops->hw_init) { >> + ret = hw_ops->hw_init(sdw); >> + if (ret) >> + return dev_err_probe(dev, ret, "hw_init failed\n"); >> + } >> + > > nit-pick: should the INIT_WORK be moved higher before this hw_init()? It > has nothing to do with regmap and we'd lose the requirement that > hw_init() be run before regmap inits. > >> INIT_WORK(&drv->boot_work, class_boot_work); >> >> dev_config->lock_arg = &drv->regmap_lock; >> @@ -222,7 +233,7 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id >> >> dev_set_drvdata(&sdw->dev, drv); >> >> - return sdca_class_probe(sdw, drv); >> + return sdca_class_probe(sdw, drv, NULL); >> } >> >> /** > ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (3 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:28 ` Pierre-Louis Bossart 2026-09-08 16:25 ` Charles Keepax 2026-09-07 8:37 ` [PATCH v2 06/11] ASoC: SDCA: class_function: xlate sound-dai cell by entity index Srinivas Kandagatla ` (5 subsequent siblings) 10 siblings, 2 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla sdca_parse_function() walks a firmware node, but on DT there is no DisCo node -- sdca_lookup_functions() is a no-op and the SDCA function descriptors come with a NULL fwnode. Add a populate_function hw_op that the class function driver falls back to when @function->desc->node is NULL: it fills the caller-owned sdca_function_data (entities, clusters, init_table, delays) from the codec's static tables, matching by function type. Leave @function->desc alone -- the framework owns the per-instance descriptor, so devices with more than one function of the same type keep their per-instance SoundWire address. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- include/sound/sdca_class.h | 6 ++++++ sound/soc/sdca/sdca_class_function.c | 9 ++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h index 3342937d09fd..3a1b6d65048f 100644 --- a/include/sound/sdca_class.h +++ b/include/sound/sdca_class.h @@ -26,9 +26,15 @@ struct sdca_function_data; * before the class regmap is created and before the slave is * ATTACHED; callers needing bus I/O must sdw_slave_wait_for_init() * first. + * @populate_function: fill @function (entities, clusters, init_table, ...) + * from static tables in place of sdca_parse_function() on + * DT/non-DisCo platforms. Must leave @function->desc alone. + * Return 0 on success or a negative errno. May be NULL. */ struct sdca_class_hw_ops { int (*hw_init)(struct sdw_slave *slave); + int (*populate_function)(struct sdw_slave *slave, + struct sdca_function_data *function); }; struct sdca_class_drv { diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c index 10a2b031b572..8bca88865a4d 100644 --- a/sound/soc/sdca/sdca_class_function.c +++ b/sound/soc/sdca/sdca_class_function.c @@ -329,7 +329,14 @@ static int class_function_probe(struct auxiliary_device *auxdev, drv->core = core; drv->function = &sdev->function; - ret = sdca_parse_function(dev, drv->function); + if (drv->function->desc->node) { + ret = sdca_parse_function(dev, drv->function); + } else if (core->hw_ops && core->hw_ops->populate_function) { + ret = core->hw_ops->populate_function(core->sdw, drv->function); + } else { + dev_err(dev, "no firmware node and no populate_function hook\n"); + return -ENOENT; + } if (ret) return ret; -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-07 8:37 ` [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data Srinivas Kandagatla @ 2026-09-07 11:28 ` Pierre-Louis Bossart 2026-09-07 13:16 ` Charles Keepax 2026-09-08 16:25 ` Charles Keepax 1 sibling, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:28 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > sdca_parse_function() walks a firmware node, but on DT there is no > DisCo node -- sdca_lookup_functions() is a no-op and the SDCA > function descriptors come with a NULL fwnode. It'd be good to have a link to an example set of DT properties for SDCA. The way the functions were organized in ACPI is based on a DT-inspired _DSD mechanism to have nested blocks. edit: After reaching the last patch I realized there are no such properties. The main problem is how to deal with board-specific initialization data, the suggestion to encode all the tables in C seems limited to me. > Add a populate_function hw_op that the class function driver falls > back to when @function->desc->node is NULL: it fills the caller-owned > sdca_function_data (entities, clusters, init_table, delays) from the > codec's static tables, matching by function type. Leave > @function->desc alone -- the framework owns the per-instance > descriptor, so devices with more than one function of the same type > keep their per-instance SoundWire address. I wasn't able to understand that last sentence - and I don't remember seeing a case with different functions of the same type. It's permitted by the spec but it'd be fun to manage... > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > include/sound/sdca_class.h | 6 ++++++ > sound/soc/sdca/sdca_class_function.c | 9 ++++++++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/sound/sdca_class.h b/include/sound/sdca_class.h > index 3342937d09fd..3a1b6d65048f 100644 > --- a/include/sound/sdca_class.h > +++ b/include/sound/sdca_class.h > @@ -26,9 +26,15 @@ struct sdca_function_data; > * before the class regmap is created and before the slave is > * ATTACHED; callers needing bus I/O must sdw_slave_wait_for_init() > * first. > + * @populate_function: fill @function (entities, clusters, init_table, ...) > + * from static tables in place of sdca_parse_function() on > + * DT/non-DisCo platforms. Must leave @function->desc alone. > + * Return 0 on success or a negative errno. May be NULL. > */ > struct sdca_class_hw_ops { > int (*hw_init)(struct sdw_slave *slave); > + int (*populate_function)(struct sdw_slave *slave, > + struct sdca_function_data *function); > }; Erm, this populate_function() callback would not generate any hardware access, would it? It's really different to hw_init which is supposed to configure the codec and must run after enumeration. I am struggling a bit with the 'hw_ops', it's really only a matter of reading tables from platform firmware, and this could be done without any dependency on hardware, couldn't it? > > struct sdca_class_drv { > diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c > index 10a2b031b572..8bca88865a4d 100644 > --- a/sound/soc/sdca/sdca_class_function.c > +++ b/sound/soc/sdca/sdca_class_function.c > @@ -329,7 +329,14 @@ static int class_function_probe(struct auxiliary_device *auxdev, > drv->core = core; > drv->function = &sdev->function; > > - ret = sdca_parse_function(dev, drv->function); > + if (drv->function->desc->node) { > + ret = sdca_parse_function(dev, drv->function); > + } else if (core->hw_ops && core->hw_ops->populate_function) { > + ret = core->hw_ops->populate_function(core->sdw, drv->function); > + } else { > + dev_err(dev, "no firmware node and no populate_function hook\n"); > + return -ENOENT; > + } > if (ret) > return ret; > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-07 11:28 ` Pierre-Louis Bossart @ 2026-09-07 13:16 ` Charles Keepax 0 siblings, 0 replies; 45+ messages in thread From: Charles Keepax @ 2026-09-07 13:16 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 01:28:42PM +0200, Pierre-Louis Bossart wrote: > On 9/7/26 10:37, Srinivas Kandagatla wrote: > > sdca_parse_function() walks a firmware node, but on DT there is no > > DisCo node -- sdca_lookup_functions() is a no-op and the SDCA > > function descriptors come with a NULL fwnode. > > It'd be good to have a link to an example set of DT properties for SDCA. > The way the functions were organized in ACPI is based on a DT-inspired > _DSD mechanism to have nested blocks. There was one in the last chain discussing this, let me know if you can't find it and I will try to dig it out my inbox. > edit: After reaching the last patch I realized there are no such > properties. The main problem is how to deal with board-specific > initialization data, the suggestion to encode all the tables in C seems > limited to me. Yeah unfortunately the DT maintainers seem to have put a hard NAK on putting SDCA properties in DT (again see previous chain for the discussion), so we are basically left with the choices of C tables as in the patch chain or using software nodes. Both will end up with the same issue of drivers eventually being fully of little quirks to tweak the tables to match the current hardware. Using software nodes is possibly more "correct" and would let us re-use the parsing, but will end up significantly more verbose. I am not sure I have a strong opinion one way or the other, apart from putting things actually in the DT would be the clearly best option. > > Add a populate_function hw_op that the class function driver falls > > back to when @function->desc->node is NULL: it fills the caller-owned > > sdca_function_data (entities, clusters, init_table, delays) from the > > codec's static tables, matching by function type. Leave > > @function->desc alone -- the framework owns the per-instance > > descriptor, so devices with more than one function of the same type > > keep their per-instance SoundWire address. > > I wasn't able to understand that last sentence - and I don't remember > seeing a case with different functions of the same type. It's permitted > by the spec but it'd be fun to manage... We have some systems that are starting to do that :-) Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-07 8:37 ` [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data Srinivas Kandagatla 2026-09-07 11:28 ` Pierre-Louis Bossart @ 2026-09-08 16:25 ` Charles Keepax 2026-09-08 18:00 ` Srinivas Kandagatla 1 sibling, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-08 16:25 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 09:37:19AM +0100, Srinivas Kandagatla wrote: > sdca_parse_function() walks a firmware node, but on DT there is no > DisCo node -- sdca_lookup_functions() is a no-op and the SDCA > function descriptors come with a NULL fwnode. > > Add a populate_function hw_op that the class function driver falls > back to when @function->desc->node is NULL: it fills the caller-owned > sdca_function_data (entities, clusters, init_table, delays) from the > codec's static tables, matching by function type. Leave > @function->desc alone -- the framework owns the per-instance > descriptor, so devices with more than one function of the same type > keep their per-instance SoundWire address. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c > index 10a2b031b572..8bca88865a4d 100644 > --- a/sound/soc/sdca/sdca_class_function.c > +++ b/sound/soc/sdca/sdca_class_function.c > @@ -329,7 +329,14 @@ static int class_function_probe(struct auxiliary_device *auxdev, > drv->core = core; > drv->function = &sdev->function; > > - ret = sdca_parse_function(dev, drv->function); > + if (drv->function->desc->node) { > + ret = sdca_parse_function(dev, drv->function); > + } else if (core->hw_ops && core->hw_ops->populate_function) { > + ret = core->hw_ops->populate_function(core->sdw, drv->function); > + } else { > + dev_err(dev, "no firmware node and no populate_function hook\n"); > + return -ENOENT; > + } Is this the right logic here? I feel like if we have a populate_function() callback we should call it regardless of if we have a node. That would help with the use-case where this is being used to work around bugs in the firmware, and populate_function() can always call sdca_parse_function() if it wants too. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-08 16:25 ` Charles Keepax @ 2026-09-08 18:00 ` Srinivas Kandagatla 2026-09-09 8:34 ` Charles Keepax 0 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 18:00 UTC (permalink / raw) To: Charles Keepax Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 5:25 PM, Charles Keepax wrote: > On Mon, Sep 07, 2026 at 09:37:19AM +0100, Srinivas Kandagatla wrote: >> sdca_parse_function() walks a firmware node, but on DT there is no >> DisCo node -- sdca_lookup_functions() is a no-op and the SDCA >> function descriptors come with a NULL fwnode. >> >> Add a populate_function hw_op that the class function driver falls >> back to when @function->desc->node is NULL: it fills the caller-owned >> sdca_function_data (entities, clusters, init_table, delays) from the >> codec's static tables, matching by function type. Leave >> @function->desc alone -- the framework owns the per-instance >> descriptor, so devices with more than one function of the same type >> keep their per-instance SoundWire address. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> >> --- >> diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c >> index 10a2b031b572..8bca88865a4d 100644 >> --- a/sound/soc/sdca/sdca_class_function.c >> +++ b/sound/soc/sdca/sdca_class_function.c >> @@ -329,7 +329,14 @@ static int class_function_probe(struct auxiliary_device *auxdev, >> drv->core = core; >> drv->function = &sdev->function; >> >> - ret = sdca_parse_function(dev, drv->function); >> + if (drv->function->desc->node) { >> + ret = sdca_parse_function(dev, drv->function); >> + } else if (core->hw_ops && core->hw_ops->populate_function) { >> + ret = core->hw_ops->populate_function(core->sdw, drv->function); >> + } else { >> + dev_err(dev, "no firmware node and no populate_function hook\n"); >> + return -ENOENT; >> + } > > Is this the right logic here? I feel like if we have a > populate_function() callback we should call it regardless of if we something like this? -------------------->cut<--------------------------------- if (drv->function->desc->node) ret = sdca_parse_function(dev, drv->function); if (core->hw_ops && core->hw_ops->populate_function) ret = core->hw_ops->populate_function(core->sdw, drv->function); if (ret) return ret; -------------------->cut<--------------------------------- > have a node. That would help with the use-case where this is > being used to work around bugs in the firmware, and > populate_function() can always call sdca_parse_function() if it > wants too. +1 --srini > > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data 2026-09-08 18:00 ` Srinivas Kandagatla @ 2026-09-09 8:34 ` Charles Keepax 0 siblings, 0 replies; 45+ messages in thread From: Charles Keepax @ 2026-09-09 8:34 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Tue, Sep 08, 2026 at 07:00:22PM +0100, Srinivas Kandagatla wrote: > On 9/8/26 5:25 PM, Charles Keepax wrote: > > On Mon, Sep 07, 2026 at 09:37:19AM +0100, Srinivas Kandagatla wrote: > >> - ret = sdca_parse_function(dev, drv->function); > >> + if (drv->function->desc->node) { > >> + ret = sdca_parse_function(dev, drv->function); > >> + } else if (core->hw_ops && core->hw_ops->populate_function) { > >> + ret = core->hw_ops->populate_function(core->sdw, drv->function); > >> + } else { > >> + dev_err(dev, "no firmware node and no populate_function hook\n"); > >> + return -ENOENT; > >> + } > > > > Is this the right logic here? I feel like if we have a > > populate_function() callback we should call it regardless of if we > > something like this? > -------------------->cut<--------------------------------- > if (drv->function->desc->node) > ret = sdca_parse_function(dev, drv->function); > > if (core->hw_ops && core->hw_ops->populate_function) > ret = core->hw_ops->populate_function(core->sdw, drv->function); > > if (ret) > return ret; I was thinking more: if (core->hw_ops && core->hw_ops->populate_function) { ret = core->hw_ops->populate_function(core->sdw, drv->function); } else if (drv->function->desc->node) { ret = sdca_parse_function(dev, drv->function); } else { dev_err(dev, "no firmware node and no populate_function hook\n"); return -ENOENT; } Although noticing as I do that it is also a little weird the populate_function() callback takes the soundwire device not the function device as an argument. I will have a look at the wider picture on that. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 06/11] ASoC: SDCA: class_function: xlate sound-dai cell by entity index 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (4 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver Srinivas Kandagatla ` (4 subsequent siblings) 10 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla sdca_asoc_populate_dais() sets each DAI id from the SDCA Function's entity array index. ASoC's default of_xlate_dai_name treats the phandle cell as a positional index into the DAI list, so on a Function with non-DAI entries between dataport entities a sound-dai phandle resolves to the wrong DAI. Register a custom of_xlate_dai_name that walks the entity array and returns the dataport entity whose array index matches the cell value. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- sound/soc/sdca/sdca_class_function.c | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c index 8bca88865a4d..ab043e23878e 100644 --- a/sound/soc/sdca/sdca_class_function.c +++ b/sound/soc/sdca/sdca_class_function.c @@ -216,9 +216,43 @@ static int class_function_set_jack(struct snd_soc_component *component, return sdca_jack_set_jack(core->irq_info, jack); } +/* + * DT phandle cell is the SDCA entity index (matches dais[].id), not the + * positional DAI index the default xlate assumes. + */ +static int class_function_of_xlate_dai_name(struct snd_soc_component *component, + const struct of_phandle_args *args, + const char **dai_name) +{ + struct class_function_drv *drv = snd_soc_component_get_drvdata(component); + struct sdca_function_data *function = drv->function; + struct sdca_entity *entity; + u32 target; + + if (args->args_count != 1) + return -EINVAL; + + target = args->args[0]; + if (target >= function->num_entities) + goto err; + + entity = &function->entities[target]; + if ((entity->type != SDCA_ENTITY_TYPE_IT && + entity->type != SDCA_ENTITY_TYPE_OT) || !entity->iot.is_dataport) + goto err; + + *dai_name = entity->label; + return 0; +err: + dev_err(component->dev, "xlate: no dataport entity at index %u (num_entities=%d)\n", + target, function->num_entities); + return -EINVAL; +} + static const struct snd_soc_component_driver class_function_component_drv = { .fixup_controls = class_function_component_fixup_controls, .remove = class_function_component_remove, + .of_xlate_dai_name = class_function_of_xlate_dai_name, .endianness = 1, }; -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (5 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 06/11] ASoC: SDCA: class_function: xlate sound-dai cell by entity index Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 8:37 ` [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void Srinivas Kandagatla ` (3 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla Add SDCA_FUNCTION_TYPE_SIMPLE_JACK to the class function driver's auxiliary id table so a SimpleJack auxdev is bound and enumerated as an ASoC component. Wire set_jack for this function type so the machine driver can register an ASoC jack against it. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- sound/soc/sdca/sdca_class_function.c | 5 +++++ sound/soc/sdca/sdca_functions.c | 2 ++ 2 files changed, 7 insertions(+) diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c index ab043e23878e..713a34d5954a 100644 --- a/sound/soc/sdca/sdca_class_function.c +++ b/sound/soc/sdca/sdca_class_function.c @@ -408,6 +408,7 @@ static int class_function_probe(struct auxiliary_device *auxdev, switch (drv->function->desc->type) { case SDCA_FUNCTION_TYPE_UAJ: case SDCA_FUNCTION_TYPE_RJ: + case SDCA_FUNCTION_TYPE_SIMPLE_JACK: cmp_drv->set_jack = class_function_set_jack; break; default: @@ -593,6 +594,10 @@ static const struct auxiliary_device_id class_function_id_table[] = { .name = "snd_soc_sdca." SDCA_FUNCTION_TYPE_RJ_NAME, .driver_data = SDCA_FUNCTION_TYPE_RJ, }, + { + .name = "snd_soc_sdca." SDCA_FUNCTION_TYPE_SIMPLE_NAME, + .driver_data = SDCA_FUNCTION_TYPE_SIMPLE_JACK, + }, {}, }; MODULE_DEVICE_TABLE(auxiliary, class_function_id_table); diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c index 47dedb62c24c..25f95575c731 100644 --- a/sound/soc/sdca/sdca_functions.c +++ b/sound/soc/sdca/sdca_functions.c @@ -79,6 +79,8 @@ static const char *get_sdca_function_name(u32 function_type) return SDCA_FUNCTION_TYPE_SPEAKER_MIC_NAME; case SDCA_FUNCTION_TYPE_RJ: return SDCA_FUNCTION_TYPE_RJ_NAME; + case SDCA_FUNCTION_TYPE_SIMPLE_JACK: + return SDCA_FUNCTION_TYPE_SIMPLE_NAME; case SDCA_FUNCTION_TYPE_COMPANION_AMP: return SDCA_FUNCTION_TYPE_COMPANION_AMP_NAME; case SDCA_FUNCTION_TYPE_IMP_DEF: -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver 2026-09-07 8:37 ` [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver Srinivas Kandagatla @ 2026-09-07 11:32 ` Pierre-Louis Bossart 0 siblings, 0 replies; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:32 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > Add SDCA_FUNCTION_TYPE_SIMPLE_JACK to the class function driver's > auxiliary id table so a SimpleJack auxdev is bound and enumerated as > an ASoC component. Wire set_jack for this function type so the machine > driver can register an ASoC jack against it. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> This should be submitted separately, there's no dependency on DT stuff? Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> > --- > sound/soc/sdca/sdca_class_function.c | 5 +++++ > sound/soc/sdca/sdca_functions.c | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c > index ab043e23878e..713a34d5954a 100644 > --- a/sound/soc/sdca/sdca_class_function.c > +++ b/sound/soc/sdca/sdca_class_function.c > @@ -408,6 +408,7 @@ static int class_function_probe(struct auxiliary_device *auxdev, > switch (drv->function->desc->type) { > case SDCA_FUNCTION_TYPE_UAJ: > case SDCA_FUNCTION_TYPE_RJ: > + case SDCA_FUNCTION_TYPE_SIMPLE_JACK: > cmp_drv->set_jack = class_function_set_jack; > break; > default: > @@ -593,6 +594,10 @@ static const struct auxiliary_device_id class_function_id_table[] = { > .name = "snd_soc_sdca." SDCA_FUNCTION_TYPE_RJ_NAME, > .driver_data = SDCA_FUNCTION_TYPE_RJ, > }, > + { > + .name = "snd_soc_sdca." SDCA_FUNCTION_TYPE_SIMPLE_NAME, > + .driver_data = SDCA_FUNCTION_TYPE_SIMPLE_JACK, > + }, > {}, > }; > MODULE_DEVICE_TABLE(auxiliary, class_function_id_table); > diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c > index 47dedb62c24c..25f95575c731 100644 > --- a/sound/soc/sdca/sdca_functions.c > +++ b/sound/soc/sdca/sdca_functions.c > @@ -79,6 +79,8 @@ static const char *get_sdca_function_name(u32 function_type) > return SDCA_FUNCTION_TYPE_SPEAKER_MIC_NAME; > case SDCA_FUNCTION_TYPE_RJ: > return SDCA_FUNCTION_TYPE_RJ_NAME; > + case SDCA_FUNCTION_TYPE_SIMPLE_JACK: > + return SDCA_FUNCTION_TYPE_SIMPLE_NAME; > case SDCA_FUNCTION_TYPE_COMPANION_AMP: > return SDCA_FUNCTION_TYPE_COMPANION_AMP_NAME; > case SDCA_FUNCTION_TYPE_IMP_DEF: ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (6 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 8:37 ` [PATCH v2 09/11] ASoC: SDCA: add sdca_apply_default_control_classifiers() helper Srinivas Kandagatla ` (2 subsequent siblings) 10 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla find_sdca_control_reset() only ever returns 0 -- it just sets control->has_reset / control->reset for the framework-known (entity type, control selector) mappings. The int return + caller error check in find_sdca_entity_control() are dead code. Convert to void and drop the check. No functional change. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- sound/soc/sdca/sdca_functions.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c index 25f95575c731..68a50552f4e4 100644 --- a/sound/soc/sdca/sdca_functions.c +++ b/sound/soc/sdca/sdca_functions.c @@ -922,8 +922,8 @@ static int find_sdca_control_value(struct device *dev, struct sdca_entity *entit return 0; } -static int find_sdca_control_reset(const struct sdca_entity *entity, - struct sdca_control *control) +static void find_sdca_control_reset(const struct sdca_entity *entity, + struct sdca_control *control) { switch (SDCA_CTL_TYPE(entity->type, control->sel)) { case SDCA_CTL_TYPE_S(FU, AGC): @@ -950,8 +950,6 @@ static int find_sdca_control_reset(const struct sdca_entity *entity, default: break; } - - return 0; } static int find_sdca_entity_control(struct device *dev, struct sdca_entity *entity, @@ -1032,9 +1030,7 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti control->is_volatile = find_sdca_control_volatile(entity, control); - ret = find_sdca_control_reset(entity, control); - if (ret) - return ret; + find_sdca_control_reset(entity, control); ret = find_sdca_control_range(dev, control_node, &control->range); if (ret) { -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void 2026-09-07 8:37 ` [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void Srinivas Kandagatla @ 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 13:03 ` Charles Keepax 0 siblings, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:32 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > find_sdca_control_reset() only ever returns 0 -- it just sets > control->has_reset / control->reset for the framework-known > (entity type, control selector) mappings. The int return + caller > error check in find_sdca_entity_control() are dead code. > > Convert to void and drop the check. No functional change. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> That looks also unrelated to DT stuff, not sure it belongs here as patch 08/11 Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> > --- > sound/soc/sdca/sdca_functions.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c > index 25f95575c731..68a50552f4e4 100644 > --- a/sound/soc/sdca/sdca_functions.c > +++ b/sound/soc/sdca/sdca_functions.c > @@ -922,8 +922,8 @@ static int find_sdca_control_value(struct device *dev, struct sdca_entity *entit > return 0; > } > > -static int find_sdca_control_reset(const struct sdca_entity *entity, > - struct sdca_control *control) > +static void find_sdca_control_reset(const struct sdca_entity *entity, > + struct sdca_control *control) > { > switch (SDCA_CTL_TYPE(entity->type, control->sel)) { > case SDCA_CTL_TYPE_S(FU, AGC): > @@ -950,8 +950,6 @@ static int find_sdca_control_reset(const struct sdca_entity *entity, > default: > break; > } > - > - return 0; > } > > static int find_sdca_entity_control(struct device *dev, struct sdca_entity *entity, > @@ -1032,9 +1030,7 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti > > control->is_volatile = find_sdca_control_volatile(entity, control); > > - ret = find_sdca_control_reset(entity, control); > - if (ret) > - return ret; > + find_sdca_control_reset(entity, control); > > ret = find_sdca_control_range(dev, control_node, &control->range); > if (ret) { ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void 2026-09-07 11:32 ` Pierre-Louis Bossart @ 2026-09-07 13:03 ` Charles Keepax 2026-09-07 13:16 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-07 13:03 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 01:32:30PM +0200, Pierre-Louis Bossart wrote: > On 9/7/26 10:37, Srinivas Kandagatla wrote: > > find_sdca_control_reset() only ever returns 0 -- it just sets > > control->has_reset / control->reset for the framework-known > > (entity type, control selector) mappings. The int return + caller > > error check in find_sdca_entity_control() are dead code. > > > > Convert to void and drop the check. No functional change. > > > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > > That looks also unrelated to DT stuff, not sure it belongs here as patch > 08/11 > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> Agree lets pull these two patches out and submit seperately as we can merge them both happily today. Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void 2026-09-07 13:03 ` Charles Keepax @ 2026-09-07 13:16 ` Srinivas Kandagatla 0 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 13:16 UTC (permalink / raw) To: Charles Keepax, Pierre-Louis Bossart Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 2:03 PM, Charles Keepax wrote: > On Mon, Sep 07, 2026 at 01:32:30PM +0200, Pierre-Louis Bossart wrote: >> On 9/7/26 10:37, Srinivas Kandagatla wrote: >>> find_sdca_control_reset() only ever returns 0 -- it just sets >>> control->has_reset / control->reset for the framework-known >>> (entity type, control selector) mappings. The int return + caller >>> error check in find_sdca_entity_control() are dead code. >>> >>> Convert to void and drop the check. No functional change. >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> >> >> That looks also unrelated to DT stuff, not sure it belongs here as patch >> 08/11 >> >> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> > > Agree lets pull these two patches out and submit seperately as we > can merge them both happily today. > Thanks Charles, Sure let me send them out separately. > Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> --srini > > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 09/11] ASoC: SDCA: add sdca_apply_default_control_classifiers() helper 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (7 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 10/11] dt-bindings: sound: qcom: add Tambora WCD9378 SDCA codec Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla 10 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla sdca_parse_function() derives is_volatile (see c7b6c6b60594) and the spec-defined reset value (see 02d851b46b36) from each Control's (entity type, selector, access mode). Codecs that ship static SDCA tables via populate_function skip that path, so RW1S action Controls end up with is_volatile=0 and get replayed on every regcache_sync. Add sdca_apply_default_control_classifiers() to re-run those two classifiers over a Function. Elevate-only, so codec-set flags win. Call from the tail of populate_function. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- include/sound/sdca_function.h | 2 ++ sound/soc/sdca/sdca_functions.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h index e580201d69d5..d3d77d8bb7cd 100644 --- a/include/sound/sdca_function.h +++ b/include/sound/sdca_function.h @@ -1464,6 +1464,8 @@ static inline u32 sdca_range_search(struct sdca_control_range *range, int sdca_parse_function(struct device *dev, struct sdca_function_data *function); +void sdca_apply_default_control_classifiers(struct sdca_function_data *function); + const char *sdca_find_terminal_name(enum sdca_terminal_type type); struct sdca_control *sdca_selector_find_control(struct device *dev, diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c index 68a50552f4e4..8f4d04e09032 100644 --- a/sound/soc/sdca/sdca_functions.c +++ b/sound/soc/sdca/sdca_functions.c @@ -2268,6 +2268,35 @@ int sdca_parse_function(struct device *dev, struct sdca_function_data *function) } EXPORT_SYMBOL_NS(sdca_parse_function, "SND_SOC_SDCA"); +/** + * sdca_apply_default_control_classifiers - apply framework-known Control fields + * @function: Function populated via static tables (populate_function path). + * + * Runs the same per-Control classifiers as sdca_parse_function() so both + * entry paths land on identical @is_volatile and @has_reset/@reset fields. + * Elevate-only: codec-set flags/values are preserved. + */ +void sdca_apply_default_control_classifiers(struct sdca_function_data *function) +{ + int i, j; + + for (i = 0; i < function->num_entities; i++) { + struct sdca_entity *entity = &function->entities[i]; + + for (j = 0; j < entity->num_controls; j++) { + struct sdca_control *control = &entity->controls[j]; + + if (!control->is_volatile) + control->is_volatile = + find_sdca_control_volatile(entity, control); + + if (!control->has_reset) + find_sdca_control_reset(entity, control); + } + } +} +EXPORT_SYMBOL_NS(sdca_apply_default_control_classifiers, "SND_SOC_SDCA"); + const char *sdca_find_terminal_name(enum sdca_terminal_type type) { switch (type) { -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 10/11] dt-bindings: sound: qcom: add Tambora WCD9378 SDCA codec 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (8 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 09/11] ASoC: SDCA: add sdca_apply_default_control_classifiers() helper Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla 10 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla Describe the WCD9378 SoundWire peripheral which is SDCA compatible soundwire device. It provides SDCA SimpleJack for headphone playback headset mic capture and MBHC jack detection. The same silicon is integrated in two ways and enumerates with the same SoundWire class ID: mobile mode: two slaves (tx, rx) on separate SoundWire masters. SDCA / compute mode: one aggregated slave on a multi-lane master. Note: both of them have different bus toplogies 1 device vs 2 devices. Currently only compute mode is supported by this bindings, mobile mode will extend this binding as required. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- .../bindings/sound/qcom,wcd9378-sdw.yaml | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd9378-sdw.yaml diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd9378-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd9378-sdw.yaml new file mode 100644 index 000000000000..9a6c309018ed --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,wcd9378-sdw.yaml @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/qcom,wcd9378-sdw.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm WCD9378 (Tambora) SoundWire codec + +maintainers: + - Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> + +description: + Qualcomm WCD9378 (Tambora) SoundWire codec in SDCA compute mode. + +properties: + compatible: + items: + - const: qcom,wcd9378c + - const: sdw20217011000 + + reg: + maxItems: 1 + + qcom,port-mapping: + description: | + Each entry maps a slave data port to a master data port. + Entries are in order starting from slave port 1; slave port 0 + is reserved and not represented. Compute mode uses 8 entries + covering DP1..DP8. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 8 + maxItems: 8 + + reset-gpios: + description: GPIO used to release the codec from reset. + maxItems: 1 + + vdd-buck-supply: + description: 1.8 V analog buck supply. + + vdd-rxtx-supply: + description: 1.8 V RX/TX supply. + + vdd-io-supply: + description: Digital I/O supply. + + vdd-mic-bias-supply: + description: Mic bias supply. + + '#sound-dai-cells': + const: 1 + +required: + - compatible + - reg + - qcom,port-mapping + - reset-gpios + - vdd-buck-supply + - vdd-rxtx-supply + - vdd-io-supply + - vdd-mic-bias-supply + - '#sound-dai-cells' + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + soundwire@7630000 { + reg = <0x07630000 0x10000>; + #address-cells = <2>; + #size-cells = <0>; + + wcd9378c_sdw: audio-codec@0,3 { + compatible = "qcom,wcd9378c", "sdw20217011000"; + reg = <0 3>; + + qcom,port-mapping = <2 2 3 4 5 6 7 8>; + + reset-gpios = <&tlmm 191 GPIO_ACTIVE_LOW>; + + vdd-buck-supply = <&vreg_l15b_1p8>; + vdd-rxtx-supply = <&vreg_l15b_1p8>; + vdd-io-supply = <&vreg_l18b_1p2>; + vdd-mic-bias-supply = <&vreg_bob1>; + + #sound-dai-cells = <1>; + }; + }; +... -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla ` (9 preceding siblings ...) 2026-09-07 8:37 ` [PATCH v2 10/11] dt-bindings: sound: qcom: add Tambora WCD9378 SDCA codec Srinivas Kandagatla @ 2026-09-07 8:37 ` Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart ` (2 more replies) 10 siblings, 3 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 8:37 UTC (permalink / raw) To: Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel, Srinivas Kandagatla Add support for the Qualcomm Tambora (WCD9378) headset codec in SDCA mode over SoundWire. On ARM/DT platforms without ACPI/DisCo firmware the SDCA topology and SoundWire port properties are supplied as static data through the codec driver. The codec exposes a single SimpleJack SDCA Function providing: - Headphone playback via FU 6 (mute + Q7.8 volume) and OT 43/45. - Headset mic capture via IT 33 with MICB2 bias derived from DT (qcom,micbias2-microvolt). - MBHC-based headset jack detection. Implements: - sdw_slave_ops.read_prop: SoundWire slave properties and dpn caps for the compute-mode dataports. - sdca_class_hw_ops.hw_init: enables supplies, toggles the reset GPIO, and enables the vendor TX PDM clock via SCP. - sdca_class_hw_ops.populate_function: fills the SDCA Function data (entities, clusters, init_table) from static tables and patches the IT 33 MIC_BIAS default with the DT-derived per-slave value. Binds SoundWire slave id 0x0217:0x0110 when qcom,compute-mode is set on the DT node. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> --- sound/soc/codecs/Kconfig | 11 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/wcd9378-sdca.c | 1010 +++++++++++++++++++++++++++++++ sound/soc/codecs/wcd9378-sdca.h | 20 + sound/soc/codecs/wcd9378-sdw.c | 52 ++ 5 files changed, 1095 insertions(+) create mode 100644 sound/soc/codecs/wcd9378-sdca.c create mode 100644 sound/soc/codecs/wcd9378-sdca.h create mode 100644 sound/soc/codecs/wcd9378-sdw.c diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index f9a47e262a77..94b6606d5e3f 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -2526,6 +2526,17 @@ config SND_SOC_WCD939X_SDW The WCD9390/9395 is a audio codec IC Integrated in Qualcomm SoCs like SM8650. +config SND_SOC_WCD9378_SDCA + tristate "Qualcomm Tambora (WCD9378) SDCA codec" + depends on SOUNDWIRE + depends on SND_SOC_SDCA_CLASS + help + This enables support for the Qualcomm Tambora (WCD9378) headset + codec when driven via the SDCA class driver on ARM platforms + without ACPI/DisCo tables. It provides the static SDCA topology + and SoundWire data port properties transcribed from the factory + ACPI tables. + config SND_SOC_WM0010 tristate depends on SPI_MASTER diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 3d122ace75ad..4d8fdd3be79b 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -366,6 +366,7 @@ snd-soc-wcd938x-y := wcd938x.o snd-soc-wcd938x-sdw-y := wcd938x-sdw.o snd-soc-wcd939x-y := wcd939x.o snd-soc-wcd939x-sdw-y := wcd939x-sdw.o +snd-soc-wcd9378-y := wcd9378-sdw.o wcd9378-sdca.o snd-soc-wm-adsp-y := wm_adsp.o snd-soc-wm-adsp-test-y := wm_adsp_fw_find_test.o snd-soc-wm0010-y := wm0010.o @@ -819,6 +820,7 @@ ifdef CONFIG_SND_SOC_WCD939X_SDW # avoid link failure by forcing sdw code built-in when needed obj-$(CONFIG_SND_SOC_WCD939X) += snd-soc-wcd939x-sdw.o endif +obj-$(CONFIG_SND_SOC_WCD9378_SDCA) += snd-soc-wcd9378.o obj-$(CONFIG_SND_SOC_WM0010) += snd-soc-wm0010.o obj-$(CONFIG_SND_SOC_WM1250_EV1) += snd-soc-wm1250-ev1.o obj-$(CONFIG_SND_SOC_WM2000) += snd-soc-wm2000.o diff --git a/sound/soc/codecs/wcd9378-sdca.c b/sound/soc/codecs/wcd9378-sdca.c new file mode 100644 index 000000000000..6a77fac01700 --- /dev/null +++ b/sound/soc/codecs/wcd9378-sdca.c @@ -0,0 +1,1010 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright (c) 2025 Qualcomm Technologies, Inc. All rights reserved. + +/* + * WCD9378 (Tambora) SDCA SimpleJack codec. Supplies the static SDCA + * topology on DT platforms where no ACPI/DisCo enumeration exists. + */ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_registers.h> +#include <linux/soundwire/sdw_type.h> +#include <linux/workqueue.h> +#include <sound/pcm.h> +#include <sound/sdca.h> +#include <sound/sdca_class.h> +#include <sound/sdca_function.h> +#include <sound/soc.h> +#include <sound/soc-dai.h> +#include "wcd9378-sdca.h" + +struct wcd9378_priv { + struct sdca_class_drv class; +}; + +static struct sdca_entity wcd9378_sdca_entities[]; +/* + * Entity array index map. Order matches the ASL entity-id-list; + * Function (Entity 0) is last. + * + * [0] E001 IT 41 (0x1) [12] E00F IT 33 (0xF) + * [1] E002 CS 41 (0x2) [13] E010 PDE 34 (0x10) + * [2] E003 MFPU 21 (0x3) [14] E011 FU 33 (0x11) + * [3] E004 XU 42 (0x4) [15] E012 SU 35 (0x12) + * [4] E007 SU 43 (0x7) [16] E013 XU 36 (0x13) + * [5] E008 SU 45 (0x8) [17] E015 CS 36 (0x15) + * [6] E009 PDE 47 (0x9) [18] E016 OT 36 (0x16) + * [7] E00A OT 43 (0xA) [19] E017 MFPU 236 (0x17) + * [8] E00B OT 45 (0xB) [20] E018 CS 236 (0x18) + * [9] E00C GE 35 (0xC) [21] E019 OT 236 (0x19) + * [10] E00D IT 131 (0xD) [22] E006 FU 6 (0x6) + * [11] E00E CS 131 (0xE) [23] E000 Function (0x0) + */ +#define QSJ_IT41 0 +#define QSJ_CS41 1 +#define QSJ_MFPU21 2 +#define QSJ_XU42 3 +#define QSJ_SU43 4 +#define QSJ_SU45 5 +#define QSJ_PDE47 6 +#define QSJ_OT43 7 +#define QSJ_OT45 8 +#define QSJ_GE35 9 +#define QSJ_IT131 10 +#define QSJ_CS131 11 +#define QSJ_IT33 12 +#define QSJ_PDE34 13 +#define QSJ_FU33 14 +#define QSJ_SU35 15 +#define QSJ_XU36 16 +#define QSJ_CS36 17 +#define QSJ_OT36 18 +#define QSJ_MFPU236 19 +#define QSJ_CS236 20 +#define QSJ_OT236 21 +#define QSJ_FU6 22 + +/* Range data: {cols, rows, data[cols*rows]}, transcribed from ASL. */ + +/* IT 41 Usage: HIFI (0x2). ULP (0x3) does not route to the HPH analog driver. */ +static u32 range_it41_usage_data[] = { + /* usage, CBN, sample_rate, sample_width, full_scale, noise_floor, tag */ + 0x2, 0x2D0, 0xBB80, 0x10, 0x0, 0x0, 0x0, +}; + +static u32 range_it41_cluster_data[] = { 0x1, 0x1 }; + +/* IT 41 DataPort selector: DP6 (HPH render). */ +static u32 range_it41_dp_data[] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x6, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF +}; + +/* CS 41 SampleRateIndex: 1 -> 48kHz PCM. */ +static u32 range_cs41_sr_data[] = { 0x1, 0xBB80 }; + +/* SU selector: disconnected / connected. */ +static u32 range_su_sel_data[] = { 0x0, 0x1 }; + +/* PDE Requested_PS: PS0 / PS3. */ +static u32 range_pde_req_ps_data[] = { 0x0, 0x3 }; + +/* + * GE 35 SelectedMode -> terminal type. + * + * Tambora MBHC only handles mechanical detection; ADC HP/HS + * discrimination is not implemented, so modes 0 (Unplugged) and 1 + * (Unknown) are aliased to Headphone (mode 4) to keep the DAPM path + * alive. Mode 2 (Line-out) is not fitted on this board. + */ +static u32 range_ge35_mode_data[] = { + 0x0, 0x6C0, /* Unplugged -> HPH (alias) */ + 0x1, 0x6C0, /* Unknown -> HPH (alias) */ + 0x3, 0x6D0, /* Headset */ + 0x4, 0x6C0, /* Headphone */ +}; + +/* IT 131 Usage: optimization render stream at 192kHz. */ +static u32 range_it131_usage_data[] = { + 0x3, 0x334, 0x2EE00, 0x8, 0x0, 0x0, 0x0, +}; + +static u32 range_it131_cluster_data[] = { 0x1, 0x3 }; + +/* IT 131 DataPort selector: DP7. */ +static u32 range_it131_dp_data[] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x7, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF +}; + +/* CS 131 SampleRateIndex: 1 -> 192kHz. */ +static u32 range_cs131_sr_data[] = { 0x1, 0x2EE00 }; + +/* IT 33 MIC_BIAS default: 2.75V (patched per-slave in populate_function). */ +static u32 range_it33_micbias_data[] = { 0x5 }; + +static u32 range_it33_usage_data[] = { + 0x1, 0x2C6, 0x0, 0x0, 0x0, 0x0, 0x0, +}; + +static u32 range_it33_cluster_data[] = { 0x1, 0x2 }; + +static u32 range_cs36_sr_data[] = { 0x1, 0xBB80 }; + +/* OT 36 Usage: PDM capture, host-visible 48kHz/16-bit PCM. */ +static u32 range_ot36_usage_data[] = { + 0x1, 0x2C6, 0xBB80, 0x10, 0x0, 0x0, 0x0, +}; + +/* OT 36 DataPort selector: DP2. */ +static u32 range_ot36_dp_data[] = { + 0xFF, 0xFF, 0x2, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF +}; + +static u32 range_cs236_sr_data[] = { 0x1, 0xBB80 }; + +/* OT 236 Usage: optimization capture at 192kHz (clocked by CS 131). */ +static u32 range_ot236_usage_data[] = { + 0x1, 0x334, 0x2EE00, 0x8, 0x0, 0x0, 0x0, +}; + +/* OT 236 DataPort selector: DP5. */ +static u32 range_ot236_dp_data[] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x5, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF +}; + +/* Entity 0 (Function) Control Values */ +static int ctrl_fun_sdca_ver_vals[] = { 0x11 }; +static int ctrl_fun_type_vals[] = { 0x08 }; /* SimpleJack */ +static int ctrl_fun_man_id_vals[] = { 0x0217 }; +static int ctrl_fun_id_vals[] = { 0x3 }; +static int ctrl_fun_ver_vals[] = { 0x0 }; +static int ctrl_dev_sdca_ver_vals[] = { 0x11 }; + +/* Entity 1 (IT 41) Control Values */ +static int ctrl_it41_latency_vals[] = { 0x0 }; +static int ctrl_it41_cluster_vals[] = { 0x1 }; +static int ctrl_it41_dp_vals[] = { 0x6 }; + +/* Entity 2 (CS 41) Control Values */ +static int ctrl_cs41_sr_vals[] = { 0x1 }; + +/* Entity 3 (MFPU 21) Control Values */ +static int ctrl_mfpu21_bypass_vals[] = { 0x1 }; + +/* Entity 4 (XU 42) Control Values */ +static int ctrl_xu42_id_vals[] = { 0x2131 }; +static int ctrl_xu42_ver_vals[] = { 0x1 }; + +/* Entity 0xD (IT 131) Control Values */ +static int ctrl_it131_latency_vals[] = { 0x0 }; +static int ctrl_it131_cluster_vals[] = { 0x1 }; +static int ctrl_it131_dp_vals[] = { 0x7 }; + +/* Entity 0xE (CS 131) Control Values */ +static int ctrl_cs131_sr_vals[] = { 0x1 }; + +/* IT 33 MIC_BIAS: fixed 2.75V (SDCA MIC_BIAS index 0x5), reapplied by PDE34. */ +static int ctrl_it33_micbias_vals[] = { 0x5 }; +static int ctrl_it33_latency_vals[] = { 0x0 }; +static int ctrl_it33_cluster_vals[] = { 0x1 }; + +/* Entity 0x13 (XU 36) Control Values */ +static int ctrl_xu36_bypass_vals[] = { 0x1 }; +static int ctrl_xu36_id_vals[] = { 0x2131 }; +static int ctrl_xu36_ver_vals[] = { 0x1 }; + +/* Entity 0x15 (CS 36) Control Values */ +static int ctrl_cs36_sr_vals[] = { 0x1 }; + +/* Entity 0x16 (OT 36) Control Values */ +static int ctrl_ot36_latency_vals[] = { 0x0 }; +static int ctrl_ot36_dp_vals[] = { 0x2 }; + +/* Entity 0x17 (MFPU 236) Control Values */ +static int ctrl_mfpu236_bypass_vals[] = { 0x1 }; + +/* Entity 0x18 (CS 236) Control Values */ +static int ctrl_cs236_sr_vals[] = { 0x1 }; + +/* Entity 0x19 (OT 236) Control Values */ +static int ctrl_ot236_latency_vals[] = { 0x0 }; +static int ctrl_ot236_dp_vals[] = { 0x5 }; + +/* Entity 0 (Function) Controls */ +static struct sdca_control entity0_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_COMMIT_GROUP_MASK_NAME }, + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_fun_sdca_ver_vals, .has_fixed = true, + .label = SDCA_CTL_FUNCTION_SDCA_VERSION_NAME }, + { .sel = 0x5, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_fun_type_vals, .has_fixed = true, + .label = SDCA_CTL_FUNCTION_TYPE_NAME }, + { .sel = 0x6, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_fun_man_id_vals, .has_fixed = true, + .label = SDCA_CTL_FUNCTION_MANUFACTURER_ID_NAME }, + { .sel = 0x7, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_fun_id_vals, .has_fixed = true, + .label = SDCA_CTL_FUNCTION_ID_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_fun_ver_vals, .has_fixed = true, + .label = SDCA_CTL_FUNCTION_VERSION_NAME }, + { .sel = 0x9, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_FUNCTION_EXTENSION_ID_NAME }, + { .sel = 0xA, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_FUNCTION_EXTENSION_VERSION_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_RW1C, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .is_volatile = true, .label = SDCA_CTL_FUNCTION_STATUS_NAME }, + { .sel = 0x11, .mode = SDCA_ACCESS_MODE_RW1S, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_FUNCTION_ACTION_NAME }, + { .sel = 0x2C, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_DEVICE_MANUFACTURER_ID_NAME }, + { .sel = 0x2D, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_DEVICE_PART_ID_NAME }, + { .sel = 0x2E, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_DEVICE_VERSION_NAME }, + { .sel = 0x2F, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_dev_sdca_ver_vals, .has_fixed = true, + .label = SDCA_CTL_DEVICE_SDCA_VERSION_NAME }, +}; + +/* Entity 1 (IT 41) Controls */ +static struct sdca_control entity_it41_controls[] = { + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x7, .rows = 0x1, .data = range_it41_usage_data }, + .label = SDCA_CTL_USAGE_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it41_latency_vals, .has_fixed = true, .label = SDCA_CTL_LATENCY_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it41_cluster_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_it41_cluster_data }, + .label = SDCA_CTL_CLUSTERINDEX_NAME }, + { .sel = 0x11, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it41_dp_vals, .has_fixed = true, + .range = { .cols = 0x10, .rows = 0x4, .data = range_it41_dp_data }, + .label = SDCA_CTL_DATAPORT_SELECTOR_NAME }, +}; + +/* Entity 2 (CS 41) Controls */ +static struct sdca_control entity_cs41_controls[] = { + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_cs41_sr_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_cs41_sr_data }, + .label = SDCA_CTL_SAMPLERATEINDEX_NAME }, +}; + +/* Entity 3 (MFPU 21) Controls */ +static struct sdca_entity *entity_mfpu21_sources[] = { + &wcd9378_sdca_entities[QSJ_IT41], + &wcd9378_sdca_entities[QSJ_IT131], +}; + +static struct sdca_control entity_mfpu21_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_mfpu21_bypass_vals, .has_fixed = true, .label = SDCA_CTL_BYPASS_NAME }, +}; + +/* Entity 4 (XU 42) Controls */ +static struct sdca_entity *entity_xu42_sources[] = { &wcd9378_sdca_entities[QSJ_MFPU21] }; + +static struct sdca_control entity_xu42_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .label = SDCA_CTL_BYPASS_NAME }, + { .sel = 0x7, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_xu42_id_vals, .has_fixed = true, .label = SDCA_CTL_XU_ID_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_xu42_ver_vals, .has_fixed = true, .label = SDCA_CTL_XU_VERSION_NAME }, +}; + +/* Entity 7 (SU 43) Controls */ +static struct sdca_entity *entity_su43_sources[] = { &wcd9378_sdca_entities[QSJ_XU42] }; + +static struct sdca_control entity_su43_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_DEVICE, + .cn_list = 0x1, + .range = { .cols = 0x1, .rows = 0x2, .data = range_su_sel_data }, + .label = SDCA_CTL_SELECTOR_NAME }, +}; + +/* Entity 8 (SU 45) Controls */ +static struct sdca_entity *entity_su45_sources[] = { &wcd9378_sdca_entities[QSJ_XU42] }; + +static struct sdca_control entity_su45_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_DEVICE, + .cn_list = 0x1, + .range = { .cols = 0x1, .rows = 0x2, .data = range_su_sel_data }, + .label = SDCA_CTL_SELECTOR_NAME }, +}; + +/* + * PDE 47 manages the HPH render path (OT 43, OT 45). FU 6 is listed + * so its cached mute/volume are reasserted on PS0 entry. + */ +static struct sdca_entity *entity_pde47_managed[] = { + &wcd9378_sdca_entities[QSJ_FU6], + &wcd9378_sdca_entities[QSJ_OT43], + &wcd9378_sdca_entities[QSJ_OT45], +}; + +static struct sdca_pde_delay pde47_delays[] = { + { .from_ps = 3, .to_ps = 0, .us = 30000 }, + { .from_ps = 0, .to_ps = 3, .us = 30000 }, +}; + +static struct sdca_control entity_pde47_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x1, .rows = 0x2, .data = range_pde_req_ps_data }, + .label = SDCA_CTL_REQUESTED_PS_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .is_volatile = true, .label = SDCA_CTL_ACTUAL_PS_NAME }, + /* + * HPH protection IRQs (OCP/CNP/SURGE) fire in codec HW but are + * not exposed to Linux; needs an SDCA framework interface for + * standalone status-bit IRQs. Wire up in a follow-up. + */ +}; + +/* OT 43 (Headphone), OT 45 (Headset): no controls. */ +static struct sdca_entity *entity_ot43_sources[] = { &wcd9378_sdca_entities[QSJ_IT41] }; +static struct sdca_entity *entity_ot45_sources[] = { &wcd9378_sdca_entities[QSJ_IT41] }; + +/* + * GE 35 mode -> SU selector. Modes 0/1 alias to Headphone (see + * range_ge35_mode_data). SU 45 = source 1 (XU 42) for HPH paths, + * SU 43 = source 1 (XU 42) for Headset. + */ +static struct sdca_ge_control ge35_mode0_controls[] = { + { .id = 0x8, .sel = 0x1, .cn = 0x0, .val = 0x1 }, +}; + +static struct sdca_ge_control ge35_mode1_controls[] = { + { .id = 0x8, .sel = 0x1, .cn = 0x0, .val = 0x1 }, +}; + +static struct sdca_ge_control ge35_mode3_controls[] = { + { .id = 0x7, .sel = 0x1, .cn = 0x0, .val = 0x1 }, +}; + +static struct sdca_ge_control ge35_mode4_controls[] = { + { .id = 0x8, .sel = 0x1, .cn = 0x0, .val = 0x1 }, +}; + +static struct sdca_ge_mode ge35_modes[] = { + { .val = 0x0, .num_controls = ARRAY_SIZE(ge35_mode0_controls), .controls = ge35_mode0_controls }, + { .val = 0x1, .num_controls = ARRAY_SIZE(ge35_mode1_controls), .controls = ge35_mode1_controls }, + { .val = 0x3, .num_controls = ARRAY_SIZE(ge35_mode3_controls), .controls = ge35_mode3_controls }, + { .val = 0x4, .num_controls = ARRAY_SIZE(ge35_mode4_controls), .controls = ge35_mode4_controls }, +}; + +static struct sdca_control entity_ge35_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .interrupt_position = SDCA_NO_INTERRUPT, + .range = { .cols = 0x2, .rows = 0x4, .data = range_ge35_mode_data }, + .label = SDCA_CTL_SELECTED_MODE_NAME }, + { .sel = 0x2, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .interrupt_position = 4, /* SDCA_4 = GE_DETECTED_MODE, see init_table INTMASK_1 */ + .is_volatile = true, .label = SDCA_CTL_DETECTED_MODE_NAME }, +}; + +static struct sdca_control entity_it131_controls[] = { + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x7, .rows = 0x1, .data = range_it131_usage_data }, + .label = SDCA_CTL_USAGE_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it131_latency_vals, .has_fixed = true, .label = SDCA_CTL_LATENCY_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it131_cluster_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_it131_cluster_data }, + .label = SDCA_CTL_CLUSTERINDEX_NAME }, + { .sel = 0x11, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it131_dp_vals, .has_fixed = true, + .range = { .cols = 0x10, .rows = 0x4, .data = range_it131_dp_data }, + .label = SDCA_CTL_DATAPORT_SELECTOR_NAME }, +}; + +/* Entity 0xE (CS 131) Controls */ +static struct sdca_control entity_cs131_controls[] = { + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_cs131_sr_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_cs131_sr_data }, + .label = SDCA_CTL_SAMPLERATEINDEX_NAME }, +}; + +/* Entity 0xF (IT 33) - headset mic input */ +static struct sdca_control entity_it33_controls[] = { + { .sel = 0x3, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it33_micbias_vals, .has_default = true, + .range = { .cols = 0x1, .rows = 0x1, .data = range_it33_micbias_data }, + .label = SDCA_CTL_MIC_BIAS_NAME }, + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x7, .rows = 0x1, .data = range_it33_usage_data }, + .has_reset = true, + .label = SDCA_CTL_USAGE_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it33_latency_vals, .has_fixed = true, .label = SDCA_CTL_LATENCY_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_it33_cluster_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_it33_cluster_data }, + .label = SDCA_CTL_CLUSTERINDEX_NAME }, +}; + +/* Entity 0x10 (PDE 34) - manages IT 33 */ +static struct sdca_entity *entity_pde34_managed[] = { &wcd9378_sdca_entities[QSJ_IT33] }; + +static struct sdca_pde_delay pde34_delays[] = { + { .from_ps = 3, .to_ps = 0, .us = 30000 }, + { .from_ps = 0, .to_ps = 3, .us = 30000 }, +}; + +static struct sdca_control entity_pde34_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x1, .rows = 0x2, .data = range_pde_req_ps_data }, + .label = SDCA_CTL_REQUESTED_PS_NAME }, + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .is_volatile = true, .label = SDCA_CTL_ACTUAL_PS_NAME }, +}; + +/* + * FU 6 (vendor FU42): HPH mute + Q7.8 volume. DUAL-mode CVR-alias + * writes take effect without an SCP_COMMIT. + */ +/* + * FU 6 volume range: MIN, MAX, STEP in Q7.8 (LSB = 1/256 dB). + * 0x8000 = -128 dB, 0x7FFF = +127.996 dB, STEP = 1. Framework + * sign-extends and converts to 0.01 dB TLV via (val * 100) >> 8. + */ +static u32 range_fu6_vol_data[] = { + 0x00008000, 0x00007FFF, 0x00000001, +}; + +static struct sdca_control entity_fu6_controls[] = { + { .sel = SDCA_CTL_FU_MUTE, .mode = SDCA_ACCESS_MODE_DUAL, + .layers = SDCA_ACCESS_LAYER_USER, .cn_list = 0x6, .nbits = 1, + .type = SDCA_CTL_DATATYPE_ONEBIT, + .label = SDCA_CTL_MUTE_NAME }, + { .sel = SDCA_CTL_FU_CHANNEL_VOLUME, .mode = SDCA_ACCESS_MODE_DUAL, + .layers = SDCA_ACCESS_LAYER_USER, .cn_list = 0x6, .nbits = 16, + .type = SDCA_CTL_DATATYPE_Q7P8DB, + .range = { .cols = SDCA_VOLUME_LINEAR_NCOLS, .rows = 1, + .data = range_fu6_vol_data }, + .label = SDCA_CTL_CHANNEL_VOLUME_NAME }, +}; + +/* Entity 0x11 (FU 33) - no controls */ +static struct sdca_entity *entity_fu33_sources[] = { &wcd9378_sdca_entities[QSJ_IT33] }; + +/* Entity 0x12 (SU 35) Controls */ +static struct sdca_entity *entity_su35_sources[] = { &wcd9378_sdca_entities[QSJ_FU33] }; + +static struct sdca_control entity_su35_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RO, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x1, .rows = 0x2, .data = range_su_sel_data }, + .label = SDCA_CTL_SELECTOR_NAME }, +}; + +/* Entity 0x13 (XU 36) Controls */ +static struct sdca_entity *entity_xu36_sources[] = { &wcd9378_sdca_entities[QSJ_SU35] }; + +static struct sdca_control entity_xu36_controls[] = { + /* bypass=1: pass mic signal through XU36 without proprietary processing */ + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_xu36_bypass_vals, .has_default = true, .label = SDCA_CTL_BYPASS_NAME }, + { .sel = 0x7, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_xu36_id_vals, .has_fixed = true, .label = SDCA_CTL_XU_ID_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_xu36_ver_vals, .has_fixed = true, .label = SDCA_CTL_XU_VERSION_NAME }, +}; + +/* Entity 0x15 (CS 36) Controls */ +static struct sdca_control entity_cs36_controls[] = { + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_cs36_sr_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_cs36_sr_data }, + .label = SDCA_CTL_SAMPLERATEINDEX_NAME }, +}; + +/* OT 36 mic capture: IT33 -> FU33 -> SU35 -> XU36 -> OT36. */ +static struct sdca_entity *entity_ot36_sources[] = { &wcd9378_sdca_entities[QSJ_XU36] }; + +static struct sdca_control entity_ot36_controls[] = { + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x7, .rows = 0x1, .data = range_ot36_usage_data }, + .label = SDCA_CTL_USAGE_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_ot36_latency_vals, .has_fixed = true, .label = SDCA_CTL_LATENCY_NAME }, + { .sel = 0x11, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_ot36_dp_vals, .has_fixed = true, + .range = { .cols = 0x10, .rows = 0x4, .data = range_ot36_dp_data }, + .label = SDCA_CTL_DATAPORT_SELECTOR_NAME }, +}; + +/* Entity 0x17 (MFPU 236) Controls */ +static struct sdca_control entity_mfpu236_controls[] = { + { .sel = 0x1, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_mfpu236_bypass_vals, .has_fixed = true, .label = SDCA_CTL_BYPASS_NAME }, +}; + +/* Entity 0x18 (CS 236) Controls */ +static struct sdca_control entity_cs236_controls[] = { + { .sel = 0x10, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_cs236_sr_vals, .has_fixed = true, + .range = { .cols = 0x2, .rows = 0x1, .data = range_cs236_sr_data }, + .label = SDCA_CTL_SAMPLERATEINDEX_NAME }, +}; + +/* Entity 0x19 (OT 236) - optimization stream capture output */ +static struct sdca_entity *entity_ot236_sources[] = { &wcd9378_sdca_entities[QSJ_IT33] }; + +static struct sdca_control entity_ot236_controls[] = { + { .sel = 0x4, .mode = SDCA_ACCESS_MODE_RW, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .range = { .cols = 0x7, .rows = 0x1, .data = range_ot236_usage_data }, + .label = SDCA_CTL_USAGE_NAME }, + { .sel = 0x8, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_ot236_latency_vals, .has_fixed = true, .label = SDCA_CTL_LATENCY_NAME }, + { .sel = 0x11, .mode = SDCA_ACCESS_MODE_DC, .layers = SDCA_ACCESS_LAYER_CLASS, .cn_list = 0x1, + .values = ctrl_ot236_dp_vals, .has_fixed = true, + .range = { .cols = 0x10, .rows = 0x4, .data = range_ot236_dp_data }, + .label = SDCA_CTL_DATAPORT_SELECTOR_NAME }, +}; + +static struct sdca_entity wcd9378_sdca_entities[] = { + /* [0] E001: IT 41 - PDM render stream input */ + { .id = 0x1, .label = "IT 41", .type = SDCA_ENTITY_TYPE_IT, + .iot = { .type = 0x0191, .is_dataport = true, .clock = &wcd9378_sdca_entities[QSJ_CS41] }, + .num_controls = ARRAY_SIZE(entity_it41_controls), .controls = entity_it41_controls }, + /* [1] E002: CS 41 */ + { .id = 0x2, .label = "CS 41", .type = SDCA_ENTITY_TYPE_CS, + .cs = { .type = 0x0 }, + .num_controls = ARRAY_SIZE(entity_cs41_controls), .controls = entity_cs41_controls }, + /* [2] E003: MFPU 21 */ + { .id = 0x3, .label = "MFPU 21", .type = SDCA_ENTITY_TYPE_MFPU, + .num_controls = ARRAY_SIZE(entity_mfpu21_controls), .controls = entity_mfpu21_controls, + .num_sources = ARRAY_SIZE(entity_mfpu21_sources), .sources = entity_mfpu21_sources }, + /* [3] E004: XU 42 */ + { .id = 0x4, .label = "XU 42", .type = SDCA_ENTITY_TYPE_XU, + .num_controls = ARRAY_SIZE(entity_xu42_controls), .controls = entity_xu42_controls, + .num_sources = ARRAY_SIZE(entity_xu42_sources), .sources = entity_xu42_sources }, + /* [4] E007: SU 43 - render selector (headset path), driven by GE 35 jack detection */ + { .id = 0x7, .label = "SU 43", .type = SDCA_ENTITY_TYPE_SU, + .group = &wcd9378_sdca_entities[QSJ_GE35], + .num_controls = ARRAY_SIZE(entity_su43_controls), .controls = entity_su43_controls, + .num_sources = ARRAY_SIZE(entity_su43_sources), .sources = entity_su43_sources }, + /* [5] E008: SU 45 - render selector (headphone path), driven by GE 35 jack detection */ + { .id = 0x8, .label = "SU 45", .type = SDCA_ENTITY_TYPE_SU, + .group = &wcd9378_sdca_entities[QSJ_GE35], + .num_controls = ARRAY_SIZE(entity_su45_controls), .controls = entity_su45_controls, + .num_sources = ARRAY_SIZE(entity_su45_sources), .sources = entity_su45_sources }, + /* [6] E009: PDE 47 - render power domain */ + { .id = 0x9, .label = "PDE 47", .type = SDCA_ENTITY_TYPE_PDE, + .pde = { .num_managed = ARRAY_SIZE(entity_pde47_managed), .managed = entity_pde47_managed, + .num_max_delay = ARRAY_SIZE(pde47_delays), .max_delay = pde47_delays }, + .num_controls = ARRAY_SIZE(entity_pde47_controls), .controls = entity_pde47_controls }, + /* [7] E00A: OT 43 - Headphone on jack */ + { .id = 0xA, .label = "OT 43", .type = SDCA_ENTITY_TYPE_OT, + .iot = { .type = 0x06C0 }, + .num_sources = ARRAY_SIZE(entity_ot43_sources), .sources = entity_ot43_sources }, + /* [8] E00B: OT 45 - Headset output on jack */ + { .id = 0xB, .label = "OT 45", .type = SDCA_ENTITY_TYPE_OT, + .iot = { .type = 0x06D0 }, + .num_sources = ARRAY_SIZE(entity_ot45_sources), .sources = entity_ot45_sources }, + /* [9] E00C: GE 35 - jack detection group entity */ + { .id = 0xC, .label = "GE 35", .type = SDCA_ENTITY_TYPE_GE, + .ge = { .num_modes = ARRAY_SIZE(ge35_modes), .modes = ge35_modes }, + .num_controls = ARRAY_SIZE(entity_ge35_controls), .controls = entity_ge35_controls }, + /* [10] E00D: IT 131 - optimization stream input */ + { .id = 0xD, .label = "IT 131", .type = SDCA_ENTITY_TYPE_IT, + .iot = { .type = 0x0190, .is_dataport = true, .clock = &wcd9378_sdca_entities[QSJ_CS131] }, + .num_controls = ARRAY_SIZE(entity_it131_controls), .controls = entity_it131_controls }, + /* [11] E00E: CS 131 */ + { .id = 0xE, .label = "CS 131", .type = SDCA_ENTITY_TYPE_CS, + .cs = { .type = 0x0 }, + .num_controls = ARRAY_SIZE(entity_cs131_controls), .controls = entity_cs131_controls }, + /* [12] E00F: IT 33 - headset mic input */ + { .id = 0xF, .label = "IT 33", .type = SDCA_ENTITY_TYPE_IT, + .iot = { .type = 0x06D0 }, + .num_controls = ARRAY_SIZE(entity_it33_controls), .controls = entity_it33_controls }, + /* [13] E010: PDE 34 - mic power domain */ + { .id = 0x10, .label = "PDE 34", .type = SDCA_ENTITY_TYPE_PDE, + .pde = { .num_managed = ARRAY_SIZE(entity_pde34_managed), .managed = entity_pde34_managed, + .num_max_delay = ARRAY_SIZE(pde34_delays), .max_delay = pde34_delays }, + .num_controls = ARRAY_SIZE(entity_pde34_controls), .controls = entity_pde34_controls }, + /* [14] E011: FU 33 - mic feature unit */ + { .id = 0x11, .label = "FU 33", .type = SDCA_ENTITY_TYPE_FU, + .num_sources = ARRAY_SIZE(entity_fu33_sources), .sources = entity_fu33_sources }, + /* [15] E012: SU 35 - mic selector */ + { .id = 0x12, .label = "SU 35", .type = SDCA_ENTITY_TYPE_SU, + .num_controls = ARRAY_SIZE(entity_su35_controls), .controls = entity_su35_controls, + .num_sources = ARRAY_SIZE(entity_su35_sources), .sources = entity_su35_sources }, + /* [16] E013: XU 36 - mic extension unit */ + { .id = 0x13, .label = "XU 36", .type = SDCA_ENTITY_TYPE_XU, + .num_controls = ARRAY_SIZE(entity_xu36_controls), .controls = entity_xu36_controls, + .num_sources = ARRAY_SIZE(entity_xu36_sources), .sources = entity_xu36_sources }, + /* [17] E015: CS 36 */ + { .id = 0x15, .label = "CS 36", .type = SDCA_ENTITY_TYPE_CS, + .cs = { .type = 0x0 }, + .num_controls = ARRAY_SIZE(entity_cs36_controls), .controls = entity_cs36_controls }, + /* [18] E016: OT 36 - PDM mic capture output */ + { .id = 0x16, .label = "OT 36", .type = SDCA_ENTITY_TYPE_OT, + .iot = { .type = 0x0191, .is_dataport = true, .clock = &wcd9378_sdca_entities[QSJ_CS36] }, + .num_controls = ARRAY_SIZE(entity_ot36_controls), .controls = entity_ot36_controls, + .num_sources = ARRAY_SIZE(entity_ot36_sources), .sources = entity_ot36_sources }, + /* [19] E017: MFPU 236 - optimization TX processing */ + { .id = 0x17, .label = "MFPU 236", .type = SDCA_ENTITY_TYPE_MFPU, + .num_controls = ARRAY_SIZE(entity_mfpu236_controls), .controls = entity_mfpu236_controls }, + /* [20] E018: CS 236 */ + { .id = 0x18, .label = "CS 236", .type = SDCA_ENTITY_TYPE_CS, + .cs = { .type = 0x0 }, + .num_controls = ARRAY_SIZE(entity_cs236_controls), .controls = entity_cs236_controls }, + /* [21] E019: OT 236 - optimization stream capture output */ + { .id = 0x19, .label = "OT 236", .type = SDCA_ENTITY_TYPE_OT, + .iot = { .type = 0x0190, .is_dataport = true, .clock = &wcd9378_sdca_entities[QSJ_CS131] }, + .num_controls = ARRAY_SIZE(entity_ot236_controls), .controls = entity_ot236_controls, + .num_sources = ARRAY_SIZE(entity_ot236_sources), .sources = entity_ot236_sources }, + /* E006: FU 6 (FU42) - vendor Feature Unit; HPH mute + Q7.8 volume. */ + { .id = 0x6, .label = "FU 6", .type = SDCA_ENTITY_TYPE_FU, + .num_controls = ARRAY_SIZE(entity_fu6_controls), .controls = entity_fu6_controls }, + /* Entity 0 (Function) */ + { .id = 0x0, .label = "entity0", + .num_controls = ARRAY_SIZE(entity0_controls), .controls = entity0_controls }, +}; + +/* Clusters */ +static struct sdca_channel cl1_channels[] = { /* CL01 - render (HPH) stereo */ + { .id = 0x1, .purpose = 0x1, .relationship = 0x2 }, /* Left */ + { .id = 0x2, .purpose = 0x1, .relationship = 0x3 }, /* Right */ +}; + +static struct sdca_channel cl2_channels[] = { /* CL02 - mic capture mono */ + { .id = 0xFF, .purpose = 0x1, .relationship = 0x1 }, +}; + +static struct sdca_channel cl3_channels[] = { /* CL03 - optimization RX */ + { .id = 0xFF, .purpose = 0x1, .relationship = 0x1 }, /* Mono */ + { .id = 0x1, .purpose = 0x1, .relationship = 0x2 }, /* Left */ + { .id = 0x2, .purpose = 0x1, .relationship = 0x3 }, /* Right */ +}; + +static struct sdca_channel cl5_channels[] = { /* CL05 - optimization TX mono */ + { .id = 0xFF, .purpose = 0x1, .relationship = 0x1 }, +}; + +static struct sdca_cluster wcd9378_sdca_clusters[] = { + { .id = 0x1, .num_channels = ARRAY_SIZE(cl1_channels), .channels = cl1_channels }, + { .id = 0x2, .num_channels = ARRAY_SIZE(cl2_channels), .channels = cl2_channels }, + { .id = 0x3, .num_channels = ARRAY_SIZE(cl3_channels), .channels = cl3_channels }, + { .id = 0x5, .num_channels = ARRAY_SIZE(cl5_channels), .channels = cl5_channels }, +}; + +/* Init table transcribed from ASL. */ +static struct sdca_init_write wcd9378_sdca_init_table[] = { + { .addr = 0x401804F0, .val = 0x00 }, /* DIGITAL_PLATFORM_CTL */ + { .addr = 0x4018046E, .val = 0x10 }, /* DIGITAL_INTR_MODE */ + { .addr = 0x0000004D, .val = 0x01 }, /* SWRS_SCP_BUSCLOCK_BASE */ + { .addr = 0x00000062, .val = 0x02 }, /* SWRS_SCP_BUSCLOCK_SCALE_BANK */ + { .addr = 0x4018016A, .val = 0x80 }, /* CP_DTOP_CTRL_14 */ + { .addr = 0x40180165, .val = 0x6b }, /* CP_DTOP_CTRL_9 */ + { .addr = 0x40180103, .val = 0x1E }, /* SLEEP_CTL BG_CTL (0.9V) */ + { .addr = 0x40180103, .val = 0x9E }, /* SLEEP_CTL BG_EN */ + { .addr = 0x40180103, .val = 0xDE }, /* SLEEP_CTL LDOL_BG_SEL */ + { .addr = 0x40180029, .val = 0xB5 }, /* BIAS_VBG_FINE_ADJ */ + { .addr = 0x40180001, .val = 0x80 }, /* ANA_BIAS ANALOG_BIAS_EN */ + { .addr = 0x40180001, .val = 0xC0 }, /* ANA_BIAS PRECHRG_EN(1) */ + { .addr = 0x40180001, .val = 0x80 }, /* ANA_BIAS PRECHRG_EN(0) */ + { .addr = 0x4018007B, .val = 0xA2 }, /* TX_COM_TXFE_DIV_CTL SEQ_BYPASS */ + { .addr = 0x40180465, .val = 0x17 }, /* PDM_WD_CTL0 TIME_OUT_SEL_PCM */ + { .addr = 0x40180466, .val = 0x17 }, /* PDM_WD_CTL1 TIME_OUT_SEL_PCM */ + { .addr = 0x4018006C, .val = 0x01 }, /* MICB1_TEST_CTL_2 IBIAS_LDO_DRIVER */ + { .addr = 0x40180072, .val = 0x81 }, /* MICB3_TEST_CTL_2 IBIAS_LDO_DRIVER */ + { .addr = 0x401800CE, .val = 0x38 }, /* HPH_OCP_CTL OCP_FSM_EN */ + { .addr = 0x401800CE, .val = 0x3A }, /* HPH_OCP_CTL SCD_OP_EN */ + { .addr = 0x401800D4, .val = 0xE1 }, /* HPH_L_TEST OCP_DET_EN */ + { .addr = 0x401800D7, .val = 0xE1 }, /* HPH_R_TEST OCP_DET_EN */ + { .addr = 0x4018044E, .val = 0x04 }, /* CDC_HPH_GAIN_CTL HPHL_RX_EN */ + { .addr = 0x4018044E, .val = 0x0C }, /* CDC_HPH_GAIN_CTL HPHR_RX_EN */ + { .addr = 0x4018000F, .val = 0x0C }, /* ANA_TX_CH2 GAIN (18.0dB) */ + { .addr = 0x40180133, .val = 0x84 }, /* HPH_NEW_INT_RDAC_HD2_CTL_L */ + { .addr = 0x40180136, .val = 0x84 }, /* HPH_NEW_INT_RDAC_HD2_CTL_R */ + { .addr = 0x401800D9, .val = 0x19 }, /* HPH_RDAC_CLK_CTL1 OPAMP_CHOP_CLK_EN */ + { .addr = 0x40180132, .val = 0x50 }, /* HPH_NEW_INT_RDAC_GAIN_CTL RDAC_GAINCTL(0.55) */ + { .addr = 0x40180510, .val = 0x05 }, /* SEQR_CTRL HPH_UP_T0 */ + { .addr = 0x40180519, .val = 0x05 }, /* SEQR_CTRL HPH_UP_T9 */ + { .addr = 0x4018051B, .val = 0x06 }, /* SEQR_CTRL HPH_DN_T0 */ + { .addr = 0x40180414, .val = 0x02 }, /* CDC_COMP_CTL_0 HPHL_COMP_EN */ + { .addr = 0x40180414, .val = 0x03 }, /* CDC_COMP_CTL_0 HPHR_COMP_EN */ + { .addr = 0x401804F2, .val = 0x80 }, /* DRE_DLY_VAL SWR_HPHL(0) */ + { .addr = 0x401804F2, .val = 0x00 }, /* DRE_DLY_VAL SWR_HPHR(0) */ + { .addr = 0x40180501, .val = 0x01 }, /* SEQR_CTRL SYS_USAGE_CTRL */ + /* Arms MBHC: jack insertion asserts SDCA_4 (GE_DETECTED_MODE). */ + { .addr = 0x40180601, .val = 0x01 }, /* MBHC_CTRL DEVICE_DET */ + { .addr = 0x40180414, .val = 0x00 }, /* CDC_COMP_CTL_0 */ + { .addr = 0x401804F2, .val = 0x88 }, /* DRE_DLY_VAL */ + { .addr = 0x40180517, .val = 0x07 }, /* SEQR_CTRL HPH_UP_T7 */ + { .addr = 0x4018051C, .val = 0x07 }, /* SEQR_CTRL HPH_DN_T1 */ + { .addr = 0x401800CE, .val = 0x28 }, /* HPH_OCP_CTL */ + { .addr = 0x401800D4, .val = 0xe0 }, /* HPH_L_TEST */ + { .addr = 0x401800D7, .val = 0xe0 }, /* HPH_R_TEST */ + { .addr = 0x40180510, .val = 0x07 }, /* SEQR_CTRL HPH_UP_T0 */ + { .addr = 0x40C80008, .val = 0x01 }, /* SMP_JACK_CTRL FUNC_ACT (RESET_FUNCTION_NOW) */ + { .addr = 0x40C00008, .val = 0x02 }, /* SMP_JACK_CTRL CMT_GRP_MASK */ + { .addr = 0x40C80000, .val = 0xFF }, /* SMP_JACK_CTRL FUNC_STAT */ + { .addr = 0x00000000, .val = 0x08 }, /* clear DP0 INT status SDCA_CASCADE */ + { .addr = 0x00000041, .val = 0x08 }, /* SCP_INT_STATUS_MASK_1 PORT_0_CASCADE_3 */ + /* Only SDCA_4 unmasked; protection IRQs stay masked. */ + { .addr = 0x0000005C, .val = 0x10 }, /* INTMASK_1 SDCA_4 (GE_DETECTED_MODE) */ + { .addr = 0x4018016A, .val = 0x00 }, /* CP_DTOP_CTRL_14 */ + { .addr = 0x40180165, .val = 0x6b }, /* CP_DTOP_CTRL_9 */ + /* + * L_DET_EN is left disabled: asserting SDCA_4 before the machine + * card registers the jack blocks the deferred card probe. + */ +}; + +/* Function Descriptor */ +static struct sdca_function_desc wcd9378_sdca_desc = { + .adr = 0x3, + .type = SDCA_FUNCTION_TYPE_SIMPLE_JACK, + .name = SDCA_FUNCTION_TYPE_SIMPLE_NAME, +}; + +/* Main Function Data */ +static struct sdca_function_data wcd9378_sdca_data = { + .desc = &wcd9378_sdca_desc, + .num_entities = ARRAY_SIZE(wcd9378_sdca_entities), + .entities = wcd9378_sdca_entities, + .num_clusters = ARRAY_SIZE(wcd9378_sdca_clusters), + .clusters = wcd9378_sdca_clusters, + .num_init_table = ARRAY_SIZE(wcd9378_sdca_init_table), + .init_table = wcd9378_sdca_init_table, + .reset_max_delay = 100000, /* 100ms — WCD9378 power-on reset completes well within this */ +}; + +/* Vendor SCP register: host clock divide-by-2 (bank 1 shadow). */ +#define WCD9378_SCP_HOST_CLK_DIV2_CTL_B1 0xF0 + +/* Slave ports 1..8, mapped to master ports via qcom,port-mapping. */ +#define WCD9378_SDCA_MAX_PORTS 8 + +static const char * const wcd9378_sdca_supplies[] = { + "vdd-buck", "vdd-rxtx", "vdd-io", "vdd-mic-bias", +}; + +int wcd9378_sdca_read_prop(struct sdw_slave *slave) +{ + struct sdw_slave_prop *prop = &slave->prop; + struct device *dev = &slave->dev; + struct sdw_dpn_prop *sink, *src; + int ret; + + ret = sdca_class_read_prop(slave); + if (ret) + return ret; + + /* Compute-mode fixed SoundWire slave properties (not described in DT) */ + prop->simple_clk_stop_capable = true; + prop->paging_support = true; + prop->clock_reg_supported = true; + prop->lane_control_support = true; + + /* Source ports: DP2 (headset mic), DP5 (optimisation TX). */ + prop->source_ports = BIT(2) | BIT(5); + /* Sink ports: DP6 (HPH audio), DP7 (HPH envelope), DP8 (optimisation RX). */ + prop->sink_ports = BIT(6) | BIT(7) | BIT(8); + + src = devm_kcalloc(dev, 2, sizeof(*src), GFP_KERNEL); + if (!src) + return -ENOMEM; + + src[0].num = 2; + src[0].type = SDW_DPN_SIMPLE; + src[0].simple_ch_prep_sm = true; + src[0].ch_prep_timeout = 10; + src[0].max_ch = 1; + src[0].min_ch = 1; + + src[1].num = 5; + src[1].type = SDW_DPN_SIMPLE; + src[1].simple_ch_prep_sm = true; + src[1].ch_prep_timeout = 10; + src[1].max_ch = 1; + src[1].min_ch = 1; + + prop->src_dpn_prop = src; + + sink = devm_kcalloc(dev, 3, sizeof(*sink), GFP_KERNEL); + if (!sink) + return -ENOMEM; + + sink[0].num = 6; + sink[0].type = SDW_DPN_SIMPLE; + sink[0].simple_ch_prep_sm = true; + sink[0].ch_prep_timeout = 10; + sink[0].max_ch = 2; + sink[0].min_ch = 1; + + sink[1].num = 7; + sink[1].type = SDW_DPN_FULL; + sink[1].simple_ch_prep_sm = true; + sink[1].ch_prep_timeout = 10; + sink[1].max_ch = 1; + sink[1].min_ch = 1; + + sink[2].num = 8; + sink[2].type = SDW_DPN_REDUCED; + sink[2].simple_ch_prep_sm = true; + sink[2].ch_prep_timeout = 10; + sink[2].max_ch = 2; + sink[2].min_ch = 1; + + prop->sink_dpn_prop = sink; + + ret = device_property_read_u32_array(dev, "qcom,port-mapping", + &slave->m_port_map[1], + WCD9378_SDCA_MAX_PORTS); + if (ret) + return dev_err_probe(dev, ret, "qcom,port-mapping missing\n"); + + return 0; +} + +static int wcd9378_sdca_hw_init(struct sdw_slave *slave) +{ + struct device *dev = &slave->dev; + struct gpio_desc *reset; + int ret; + + /* No SPMI parent: supplies and reset live on the SoundWire DT node. */ + ret = devm_regulator_bulk_get_enable(dev, + ARRAY_SIZE(wcd9378_sdca_supplies), + wcd9378_sdca_supplies); + if (ret) + return dev_err_probe(dev, ret, "failed to enable supplies\n"); + + reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(reset)) + return dev_err_probe(dev, PTR_ERR(reset), + "failed to get reset GPIO\n"); + + if (reset) { + gpiod_set_value(reset, 1); + usleep_range(20, 30); + gpiod_set_value(reset, 0); + usleep_range(20, 30); + } + + /* SCP writes below need the slave attached. */ + ret = sdw_slave_wait_for_init(slave, 5000); + if (ret) + return dev_err_probe(dev, ret, + "slave attach timeout: %d\n", ret); + + /* + * TX PDM clock: bank-1 shadow + SCP_COMMIT. SCP survives PDE + * cycles; one-shot at hw_init before any port is enabled. + */ + ret = sdw_write_no_pm(slave, WCD9378_SCP_HOST_CLK_DIV2_CTL_B1, 0x01); + if (ret) + return dev_err_probe(dev, ret, + "HOST_CLK_DIV2_CTL_B1: %d\n", ret); + + ret = sdw_write_no_pm(slave, SDW_SCP_COMMIT, 0x02); + if (ret) + return dev_err_probe(dev, ret, + "SCP_COMMIT: %d\n", ret); + + return 0; +} + +static int wcd9378_sdca_populate_function(struct sdw_slave *slave, + struct sdca_function_data *function) +{ + /* @function->desc is already set by the framework; fill payload only. */ + if (function->desc->type != wcd9378_sdca_desc.type) + return -EINVAL; + + function->num_entities = wcd9378_sdca_data.num_entities; + function->entities = wcd9378_sdca_data.entities; + function->num_clusters = wcd9378_sdca_data.num_clusters; + function->clusters = wcd9378_sdca_data.clusters; + function->num_init_table = wcd9378_sdca_data.num_init_table; + function->init_table = wcd9378_sdca_data.init_table; + function->reset_max_delay = wcd9378_sdca_data.reset_max_delay; + + /* Elevate is_volatile / has_reset to match the DisCo/ACPI path. */ + sdca_apply_default_control_classifiers(function); + + return 0; +} + +static const struct sdca_class_hw_ops wcd9378_sdca_hw_ops = { + .hw_init = wcd9378_sdca_hw_init, + .populate_function = wcd9378_sdca_populate_function, +}; + +int wcd9378_sdca_probe(struct sdw_slave *slave, + const struct sdw_device_id *id) +{ + struct device *dev = &slave->dev; + struct sdca_device_data *data = &slave->sdca_data; + struct wcd9378_priv *priv; + + /* + * 0x0217:0x0110 covers both mobile and compute modes; the + * qcom,wcd9378c variant compatible identifies compute-mode + * nodes only. Mobile-mode nodes carry the plain class-ID + * compatible and are picked up by the mobile driver. + */ + if (!device_is_compatible(dev, "qcom,wcd9378c")) + return -ENODEV; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + dev_set_drvdata(dev, priv); + + /* DT has no DisCo enumeration; seed the descriptor here. */ + if (!data->num_functions) { + data->function[0].type = wcd9378_sdca_desc.type; + data->function[0].adr = wcd9378_sdca_desc.adr; + data->function[0].name = wcd9378_sdca_desc.name; + data->num_functions = 1; + } + + return sdca_class_probe(slave, &priv->class, &wcd9378_sdca_hw_ops); +} + +void wcd9378_sdca_remove(struct sdw_slave *slave) +{ + struct wcd9378_priv *priv = dev_get_drvdata(&slave->dev); + + sdca_class_remove(&priv->class); +} + +int wcd9378_sdca_runtime_suspend(struct device *dev) +{ + struct wcd9378_priv *priv = dev_get_drvdata(dev); + + return sdca_class_runtime_suspend(&priv->class); +} + +int wcd9378_sdca_runtime_resume(struct device *dev) +{ + struct wcd9378_priv *priv = dev_get_drvdata(dev); + + return sdca_class_runtime_resume(&priv->class); +} + +int wcd9378_sdca_system_suspend(struct device *dev) +{ + struct wcd9378_priv *priv = dev_get_drvdata(dev); + + return sdca_class_system_suspend(&priv->class); +} + +int wcd9378_sdca_system_resume(struct device *dev) +{ + struct wcd9378_priv *priv = dev_get_drvdata(dev); + + return sdca_class_system_resume(&priv->class); +} + +/* SoundWire slave-driver plumbing lives in wcd9378-sdw.c. */ diff --git a/sound/soc/codecs/wcd9378-sdca.h b/sound/soc/codecs/wcd9378-sdca.h new file mode 100644 index 000000000000..6ddae9da395b --- /dev/null +++ b/sound/soc/codecs/wcd9378-sdca.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* Copyright (c) 2025 Qualcomm Technologies, Inc. */ + +#ifndef _WCD9378_SDCA_H +#define _WCD9378_SDCA_H + +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_type.h> + +int wcd9378_sdca_probe(struct sdw_slave *slave, + const struct sdw_device_id *id); +void wcd9378_sdca_remove(struct sdw_slave *slave); +int wcd9378_sdca_read_prop(struct sdw_slave *slave); + +int wcd9378_sdca_runtime_suspend(struct device *dev); +int wcd9378_sdca_runtime_resume(struct device *dev); +int wcd9378_sdca_system_suspend(struct device *dev); +int wcd9378_sdca_system_resume(struct device *dev); + +#endif /* _WCD9378_SDCA_H */ diff --git a/sound/soc/codecs/wcd9378-sdw.c b/sound/soc/codecs/wcd9378-sdw.c new file mode 100644 index 000000000000..4e03bb2d9509 --- /dev/null +++ b/sound/soc/codecs/wcd9378-sdw.c @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright (c) 2025 Qualcomm Technologies, Inc. + +/* + * WCD9378 (Tambora) SoundWire driver glue for the SDCA compute-mode + * codec. The SDCA topology and hardware-specific probe logic live in + * wcd9378-sdca.c; this file carries the SoundWire slave driver plumbing + * and the module boilerplate. + */ + +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_type.h> +#include "wcd9378-sdca.h" + +static const struct dev_pm_ops wcd9378_sdw_pm_ops = { + SYSTEM_SLEEP_PM_OPS(wcd9378_sdca_system_suspend, + wcd9378_sdca_system_resume) + RUNTIME_PM_OPS(wcd9378_sdca_runtime_suspend, + wcd9378_sdca_runtime_resume, NULL) +}; + +static const struct sdw_slave_ops wcd9378_sdw_ops = { + .read_prop = wcd9378_sdca_read_prop, +}; + +static const struct sdw_device_id wcd9378_sdw_id[] = { + SDW_SLAVE_ENTRY(0x0217, 0x0110, 0), + {} +}; +MODULE_DEVICE_TABLE(sdw, wcd9378_sdw_id); + +static struct sdw_driver wcd9378_sdw_driver = { + .driver = { + .name = "wcd9378", + .pm = pm_ptr(&wcd9378_sdw_pm_ops), + }, + .probe = wcd9378_sdca_probe, + .remove = wcd9378_sdca_remove, + .id_table = wcd9378_sdw_id, + .ops = &wcd9378_sdw_ops, +}; +module_sdw_driver(wcd9378_sdw_driver); + +MODULE_DESCRIPTION("Qualcomm WCD9378 (Tambora) SoundWire codec"); +MODULE_AUTHOR("Qualcomm Technologies, Inc."); +MODULE_LICENSE("Dual BSD/GPL"); +MODULE_IMPORT_NS("SND_SOC_SDCA"); +MODULE_IMPORT_NS("SND_SOC_SDCA_CLASS"); -- 2.53.0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla @ 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 13:03 ` Srinivas Kandagatla 2026-09-08 15:58 ` Uwe Kleine-König 2026-09-08 16:20 ` Charles Keepax 2 siblings, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 11:32 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 10:37, Srinivas Kandagatla wrote: > Add support for the Qualcomm Tambora (WCD9378) headset codec in SDCA > mode over SoundWire. On ARM/DT platforms without ACPI/DisCo firmware > the SDCA topology and SoundWire port properties are supplied as static > data through the codec driver. wow, I realize now I completely misunderstood what this whole endeavor was about. I *thought* the point was to read the information about all the functions from Device Tree tables. I now understand there are no such tables, all the information is encoded in C as part of the higher-level codec driver. Is this really intended? I mean, the whole ACPI set of definitions relied on the _DSD mechanism that mimics what DT provides. Do we really want all this information in C? Why not have a set of DT properties for each function? I guess my main objection is for opaque initialization data aka blind writes or SWF table, this should really come from platform firmware, no? With this approach you'd have an endless set of kernel quirks for each board variant using the same codec. > +static int wcd9378_sdca_hw_init(struct sdw_slave *slave) > +{ > + struct device *dev = &slave->dev; > + struct gpio_desc *reset; > + int ret; > + > + /* No SPMI parent: supplies and reset live on the SoundWire DT node. */ > + ret = devm_regulator_bulk_get_enable(dev, > + ARRAY_SIZE(wcd9378_sdca_supplies), > + wcd9378_sdca_supplies); > + if (ret) > + return dev_err_probe(dev, ret, "failed to enable supplies\n"); > + > + reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(reset)) > + return dev_err_probe(dev, PTR_ERR(reset), > + "failed to get reset GPIO\n"); > + > + if (reset) { > + gpiod_set_value(reset, 1); > + usleep_range(20, 30); > + gpiod_set_value(reset, 0); > + usleep_range(20, 30); > + } Could this be part of wcd9378_sdca_probe() > + > + /* SCP writes below need the slave attached. */ > + ret = sdw_slave_wait_for_init(slave, 5000); > + if (ret) > + return dev_err_probe(dev, ret, > + "slave attach timeout: %d\n", ret); That wait doesn't seem required, if you are using the existing class probe there's already a wait? > + > + /* > + * TX PDM clock: bank-1 shadow + SCP_COMMIT. SCP survives PDE > + * cycles; one-shot at hw_init before any port is enabled. > + */ > + ret = sdw_write_no_pm(slave, WCD9378_SCP_HOST_CLK_DIV2_CTL_B1, 0x01); > + if (ret) > + return dev_err_probe(dev, ret, > + "HOST_CLK_DIV2_CTL_B1: %d\n", ret); > + > + ret = sdw_write_no_pm(slave, SDW_SCP_COMMIT, 0x02); > + if (ret) > + return dev_err_probe(dev, ret, > + "SCP_COMMIT: %d\n", ret); > + > + return 0; and this could also be done in the existing .status callback upon enumeration. In other words the need for this hw_init() isn't very clear to me... > +} > + > +static int wcd9378_sdca_populate_function(struct sdw_slave *slave, > + struct sdca_function_data *function) > +{ > + /* @function->desc is already set by the framework; fill payload only. */ > + if (function->desc->type != wcd9378_sdca_desc.type) > + return -EINVAL; > + > + function->num_entities = wcd9378_sdca_data.num_entities; > + function->entities = wcd9378_sdca_data.entities; > + function->num_clusters = wcd9378_sdca_data.num_clusters; > + function->clusters = wcd9378_sdca_data.clusters; > + function->num_init_table = wcd9378_sdca_data.num_init_table; > + function->init_table = wcd9378_sdca_data.init_table; > + function->reset_max_delay = wcd9378_sdca_data.reset_max_delay; > + > + /* Elevate is_volatile / has_reset to match the DisCo/ACPI path. */ > + sdca_apply_default_control_classifiers(function); > + > + return 0; > +} > + > +static const struct sdca_class_hw_ops wcd9378_sdca_hw_ops = { > + .hw_init = wcd9378_sdca_hw_init, > + .populate_function = wcd9378_sdca_populate_function, > +}; > + > +int wcd9378_sdca_probe(struct sdw_slave *slave, > + const struct sdw_device_id *id) > +{ > + struct device *dev = &slave->dev; > + struct sdca_device_data *data = &slave->sdca_data; > + struct wcd9378_priv *priv; > + > + /* > + * 0x0217:0x0110 covers both mobile and compute modes; the > + * qcom,wcd9378c variant compatible identifies compute-mode > + * nodes only. Mobile-mode nodes carry the plain class-ID > + * compatible and are picked up by the mobile driver. > + */ > + if (!device_is_compatible(dev, "qcom,wcd9378c")) > + return -ENODEV; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + dev_set_drvdata(dev, priv); > + > + /* DT has no DisCo enumeration; seed the descriptor here. */ > + if (!data->num_functions) { > + data->function[0].type = wcd9378_sdca_desc.type; > + data->function[0].adr = wcd9378_sdca_desc.adr; > + data->function[0].name = wcd9378_sdca_desc.name; > + data->num_functions = 1; > + } > + > + return sdca_class_probe(slave, &priv->class, &wcd9378_sdca_hw_ops); > +} ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 11:32 ` Pierre-Louis Bossart @ 2026-09-07 13:03 ` Srinivas Kandagatla 2026-09-07 19:47 ` Pierre-Louis Bossart 0 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 13:03 UTC (permalink / raw) To: Pierre-Louis Bossart, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel Thanks Pierre for review, On 9/7/26 12:32 PM, Pierre-Louis Bossart wrote: > On 9/7/26 10:37, Srinivas Kandagatla wrote: >> Add support for the Qualcomm Tambora (WCD9378) headset codec in SDCA >> mode over SoundWire. On ARM/DT platforms without ACPI/DisCo firmware >> the SDCA topology and SoundWire port properties are supplied as static >> data through the codec driver. > > wow, I realize now I completely misunderstood what this whole endeavor > was about. I *thought* the point was to read the information about all > the functions from Device Tree tables. I now understand there are no > such tables, all the information is encoded in C as part of the > higher-level codec driver. > > Is this really intended? Unfortunately Yes. > I mean, the whole ACPI set of definitions relied on the _DSD mechanism > that mimics what DT provides. Do we really want all this information in > C? Why not have a set of DT properties for each function? > We discussed this topic at LPC 2025, Devicetree MC: "DeviceTrees - MIPI SoundWire Device Class for Audio (SDCA) and classic ACPI-DT problem" https://lpc.events/event/19/contributions/2024/ Among other options presented, representing them in a intermediate format was something which was doable. I have also proposed another follow up of this topic in this years LPC Devicetree MC too RFC of this patchset got some comments from DT maitainers. DT maintainers are not happy with the idea of keeping this info in DT while it can be derived from compatible string. https://lkml.org/lkml/2026/7/29/1166 > I guess my main objection is for opaque initialization data aka blind > writes or SWF table, this should really come from platform firmware, no? This table is directly generated from ACPI tables both from Lenovo T14 and Reference platform. on ARM platforms DT is is the only firmware entry for such things and its not 1:1 with ACPI example, Somethings that can be derived can not be in Device tree description so its bit of mix. > With this approach you'd have an endless set of kernel quirks for each > board variant using the same codec. @Krzysztof Kozlowski <krzk+dt@kernel.org> that is a valid point. Idea is to gate them using platform specific compatibles, so far we have few laptops that are pretty much identical w.r.t the description, may be we got lucky in this early stages. In future if it turns out to need a quirks per platform or changes needed in this table then we should be able to handle it with platform specific device compatibles. >> +static int wcd9378_sdca_hw_init(struct sdw_slave *slave) >> +{ >> + struct device *dev = &slave->dev; >> + struct gpio_desc *reset; >> + int ret; >> + >> + /* No SPMI parent: supplies and reset live on the SoundWire DT node. */ >> + ret = devm_regulator_bulk_get_enable(dev, >> + ARRAY_SIZE(wcd9378_sdca_supplies), >> + wcd9378_sdca_supplies); >> + if (ret) >> + return dev_err_probe(dev, ret, "failed to enable supplies\n"); >> + >> + reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); >> + if (IS_ERR(reset)) >> + return dev_err_probe(dev, PTR_ERR(reset), >> + "failed to get reset GPIO\n"); >> + >> + if (reset) { >> + gpiod_set_value(reset, 1); >> + usleep_range(20, 30); >> + gpiod_set_value(reset, 0); >> + usleep_range(20, 30); >> + } > > Could this be part of wcd9378_sdca_probe() Good point, I have now moved this to probe and eliminates need of hw_init callback all together, I can fold that in next version. --srini >> + >> + /* SCP writes below need the slave attached. */ >> + ret = sdw_slave_wait_for_init(slave, 5000); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "slave attach timeout: %d\n", ret); > > That wait doesn't seem required, if you are using the existing class > probe there's already a wait? >> + >> + /* >> + * TX PDM clock: bank-1 shadow + SCP_COMMIT. SCP survives PDE >> + * cycles; one-shot at hw_init before any port is enabled. >> + */ >> + ret = sdw_write_no_pm(slave, WCD9378_SCP_HOST_CLK_DIV2_CTL_B1, 0x01); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "HOST_CLK_DIV2_CTL_B1: %d\n", ret); >> + >> + ret = sdw_write_no_pm(slave, SDW_SCP_COMMIT, 0x02); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "SCP_COMMIT: %d\n", ret); >> + >> + return 0; > > and this could also be done in the existing .status callback upon > enumeration. > > In other words the need for this hw_init() isn't very clear to me... > >> +} >> + >> +static int wcd9378_sdca_populate_function(struct sdw_slave *slave, >> + struct sdca_function_data *function) >> +{ >> + /* @function->desc is already set by the framework; fill payload only. */ >> + if (function->desc->type != wcd9378_sdca_desc.type) >> + return -EINVAL; >> + >> + function->num_entities = wcd9378_sdca_data.num_entities; >> + function->entities = wcd9378_sdca_data.entities; >> + function->num_clusters = wcd9378_sdca_data.num_clusters; >> + function->clusters = wcd9378_sdca_data.clusters; >> + function->num_init_table = wcd9378_sdca_data.num_init_table; >> + function->init_table = wcd9378_sdca_data.init_table; >> + function->reset_max_delay = wcd9378_sdca_data.reset_max_delay; >> + >> + /* Elevate is_volatile / has_reset to match the DisCo/ACPI path. */ >> + sdca_apply_default_control_classifiers(function); >> + >> + return 0; >> +} >> + >> +static const struct sdca_class_hw_ops wcd9378_sdca_hw_ops = { >> + .hw_init = wcd9378_sdca_hw_init, >> + .populate_function = wcd9378_sdca_populate_function, >> +}; >> + >> +int wcd9378_sdca_probe(struct sdw_slave *slave, >> + const struct sdw_device_id *id) >> +{ >> + struct device *dev = &slave->dev; >> + struct sdca_device_data *data = &slave->sdca_data; >> + struct wcd9378_priv *priv; >> + >> + /* >> + * 0x0217:0x0110 covers both mobile and compute modes; the >> + * qcom,wcd9378c variant compatible identifies compute-mode >> + * nodes only. Mobile-mode nodes carry the plain class-ID >> + * compatible and are picked up by the mobile driver. >> + */ >> + if (!device_is_compatible(dev, "qcom,wcd9378c")) >> + return -ENODEV; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + dev_set_drvdata(dev, priv); >> + >> + /* DT has no DisCo enumeration; seed the descriptor here. */ >> + if (!data->num_functions) { >> + data->function[0].type = wcd9378_sdca_desc.type; >> + data->function[0].adr = wcd9378_sdca_desc.adr; >> + data->function[0].name = wcd9378_sdca_desc.name; >> + data->num_functions = 1; >> + } >> + >> + return sdca_class_probe(slave, &priv->class, &wcd9378_sdca_hw_ops); >> +} ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 13:03 ` Srinivas Kandagatla @ 2026-09-07 19:47 ` Pierre-Louis Bossart 2026-09-07 21:26 ` Mark Brown 2026-09-07 22:37 ` Srinivas Kandagatla 0 siblings, 2 replies; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-07 19:47 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel >> wow, I realize now I completely misunderstood what this whole endeavor >> was about. I *thought* the point was to read the information about all >> the functions from Device Tree tables. I now understand there are no >> such tables, all the information is encoded in C as part of the >> higher-level codec driver. >> >> Is this really intended? > Unfortunately Yes. > >> I mean, the whole ACPI set of definitions relied on the _DSD mechanism >> that mimics what DT provides. Do we really want all this information in >> C? Why not have a set of DT properties for each function? >> > > We discussed this topic at LPC 2025, Devicetree MC: > "DeviceTrees - MIPI SoundWire Device Class for Audio (SDCA) and > classic ACPI-DT problem" > https://lpc.events/event/19/contributions/2024/ > > Among other options presented, representing them in a intermediate > format was something which was doable. > > I have also proposed another follow up of this topic in this years LPC > Devicetree MC too > > RFC of this patchset got some comments from DT maitainers. > > DT maintainers are not happy with the idea of keeping this info in DT > while it can be derived from compatible string. > > https://lkml.org/lkml/2026/7/29/1166 > >> I guess my main objection is for opaque initialization data aka blind >> writes or SWF table, this should really come from platform firmware, no? > This table is directly generated from ACPI tables both from Lenovo T14 > and Reference platform. > > on ARM platforms DT is is the only firmware entry for such things and > its not 1:1 with ACPI example, Somethings that can be derived can not be > in Device tree description so its bit of mix. > >> With this approach you'd have an endless set of kernel quirks for each >> board variant using the same codec. > > @Krzysztof Kozlowski <krzk+dt@kernel.org> that is a valid point. > > Idea is to gate them using platform specific compatibles, so far we have > few laptops that are pretty much identical w.r.t the description, may be > we got lucky in this early stages. > > In future if it turns out to need a quirks per platform or changes > needed in this table then we should be able to handle it with platform > specific device compatibles. Thanks for the pointers. Unfortunately I missed all these threads but I don't have the background to understand the difference between software nodes, secondary fwnodes, etc. What I can tell you is that the SDCA definitions were NOT designed for ACPI. We used the concepts we wanted to expose to drivers and the Intel ACPI folks told us how to represent them in ACPI. At the time, we didn't have *anyone* in the WG who could speak for DT platforms, and the main push was Windows platforms so ACPI became the path of least resistance to get things done. The entire SDCA document is a design-by-committee exercise where we tried to make progress, you need to look at the SDCA document and not think "how do I translate the properties to DT syntax' but 'how do I extract useful concepts and represent them well in DT'... I could go on forever on what happened since 2017 but here goes: 1) One of the most frustrating things with ACPI was the lack of 'subblocks', we had to define indirections using "_DSD Hierarchical Data Extension Properties", which wasn't a walk in the park. That makes the SDCA text file completely unreadable for humans, and to the best of my knowledge no one ever released tools. This was the case when the properties were integer values, but now that the properties include binary arrays it's even worse. You could do much better with DT... 2) The other problem is that some of the DisCo and SDCA properties are not generic, they only make sense in the context of a specific board. For example even if a codec is capable of detecting jacks, platform firmware could be modified to remove this capability. The delays or power states supported could be different between multiple boards. IOW, the DisCo information does make provide information on codec v. board context, it's all board. It's a bit silly but at the time we didn't have an ACPI way of representing a board - it's available now with the ACPI0018 'audio composition' device but too late... 3) the initial plan was to represent everything in DisCo, but that went sideways when it became allowed to use firmware download for tables, and when a separate SWFT format was invented. That provides 3 ways of storing opaque/binary data... 4) and perhaps the most confusing for me, the codec hardware is allowed to request specific binary data for specific uses, which breaks further the difference between codec and board... My take is that rather than encode all properties in C, it's probably worth exploring a DT representation of the concepts that *can* be used fairly easily and make the life of codec vendors easier - not as a literal translation of ACPI. Putting everything in C seems like a code management nightmare to me. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 19:47 ` Pierre-Louis Bossart @ 2026-09-07 21:26 ` Mark Brown 2026-09-07 22:37 ` Srinivas Kandagatla 1 sibling, 0 replies; 45+ messages in thread From: Mark Brown @ 2026-09-07 21:26 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, Rob Herring, Charles Keepax, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 914 bytes --] On Mon, Sep 07, 2026 at 09:47:42PM +0200, Pierre-Louis Bossart wrote: > My take is that rather than encode all properties in C, it's probably > worth exploring a DT representation of the concepts that *can* be used > fairly easily and make the life of codec vendors easier - not as a > literal translation of ACPI. > Putting everything in C seems like a code management nightmare to me. I think one of the issues they have here is that there's two cases that need handling. There's the case of DT native platforms where the vendor can write a sensible DT, but there's also the case of platforms that are ACPI native with ACPI that is basically unusable by Linux since it relies very heavily on being supplimented by a platform specific driver blob. On those platforms there is little to no vendor support for writing DT and you're stuck with some combination of reverse engineering and reusing the ACPI data. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 19:47 ` Pierre-Louis Bossart 2026-09-07 21:26 ` Mark Brown @ 2026-09-07 22:37 ` Srinivas Kandagatla 2026-09-08 8:49 ` Charles Keepax 1 sibling, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-07 22:37 UTC (permalink / raw) To: Pierre-Louis Bossart, Mark Brown, Rob Herring, Charles Keepax Cc: Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: > My take is that rather than encode all properties in C, it's probably > worth exploring a DT representation of the concepts that *can* be used > fairly easily and make the life of codec vendors easier - not as a> literal translation of ACPI. Two things are getting mixed here: A. Representation: how the SDCA info is expressed as ACPI DisCo, DT properties, static C tables, or a firmware blob. B. Reuse of the generic class function drivers, which ultimately consume struct sdca_function_data. Whatever A looks like, we end up at the same struct sdca_function_data that B consumes. sdca_parse_function() converts ACPI DisCo to it today; populate_function() does the same from static C tables; a DT parser would do it from DT. Same runtime after that. Static C tables at least mirror the SDCA spec's own entity/control structure, so the code stays readable against the spec rather than inventing a new shape. They are also a byte-for-byte match against Qualcomm's QcSimpleJack.asl and a live Lenovo T14s DSDT, so we are not maintaining them as a fork of the vendor source, they are a mechanical transcription with a verifiable origin. We landed on static C tables here because the previous DT round was clear: "if it's derivable from the compat string, keep it out of DT" (https://lkml.org/lkml/2026/7/29/1166). As Mark pointed out, DT + SDCA has two use-cases today: 1. Devices that have a reference ACPI table but boot Linux with DT because no ACPI enablement package (PEP) exists on the SoC yet. this is what I'm working on. 2. Devices with no ACPI reference at all. I'm not sure any of the SDCA codecs in sound/soc/codecs/*sdca* today falls into that category. Solving both in one shot requires SDCA DT bindings that either translate to struct sdca_function_data, or represent the SDCA spec fully in DT and share the same parser flow. I agree a DT expression of the *concepts* not a literal DisCo mirror would be a better A. I want to bring that back to LPC 2026 DT MC as a concrete proposal grounded in the earlier DT-maintainer feedback rather than re-litigating it inline on this series. Because B stays put, switching A later is a change inside populate_function, not an ABI break. so landing this series does not close the door on the representation you're asking for. --srini > > Putting everything in C seems like a code management nightmare to me. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 22:37 ` Srinivas Kandagatla @ 2026-09-08 8:49 ` Charles Keepax 2026-09-08 9:09 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-08 8:49 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: > On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: > > My take is that rather than encode all properties in C, it's probably > > worth exploring a DT representation of the concepts that *can* be used > > fairly easily and make the life of codec vendors easier - not as a> literal translation of ACPI. > > I agree a DT expression of the *concepts* not a literal DisCo > mirror would be a better A. I want to bring that back to LPC 2026 > DT MC as a concrete proposal grounded in the earlier DT-maintainer > feedback rather than re-litigating it inline on this series. Because > B stays put, switching A later is a change inside populate_function, > not an ABI break. so landing this series does not close the door on > the representation you're asking for. I am still not sure I really see what the value is in a completely different DT representation of the DisCo information, all it does is give us potential issues with future spec versions and create a whole bunch of new code than needs maintained. The ACPI representation translates perfectly well to DT, should work perfectly fine using the existing code. However, that said if people want to work on doing so I am not going to stand in the way. It does have the advantage it might unblock things with the DT maintainers, since we can focus on doing stuff in a more traditional DT style. I would also agree that this is reasonable as a half way house, it seems like a good fallback mechanism to have for emergency work arounds as well. If we can't reuse the existing parser then there is a lot of work to define a new representation for DT. Ideally that would be worked through with the MIPI group as well to avoid future specs causing catastrophic issues. But all that will take significant time so we probably need an interim solution. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 8:49 ` Charles Keepax @ 2026-09-08 9:09 ` Srinivas Kandagatla 2026-09-08 10:37 ` Richard Fitzgerald 0 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 9:09 UTC (permalink / raw) To: Charles Keepax Cc: Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 9:49 AM, Charles Keepax wrote: > On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: >> On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: >>> My take is that rather than encode all properties in C, it's probably >>> worth exploring a DT representation of the concepts that *can* be used >>> fairly easily and make the life of codec vendors easier - not as a> literal translation of ACPI. >> >> I agree a DT expression of the *concepts* not a literal DisCo >> mirror would be a better A. I want to bring that back to LPC 2026 >> DT MC as a concrete proposal grounded in the earlier DT-maintainer >> feedback rather than re-litigating it inline on this series. Because >> B stays put, switching A later is a change inside populate_function, >> not an ABI break. so landing this series does not close the door on >> the representation you're asking for. > > I am still not sure I really see what the value is in a > completely different DT representation of the DisCo information, > all it does is give us potential issues with future spec versions > and create a whole bunch of new code than needs maintained. The > ACPI representation translates perfectly well to DT, should work > perfectly fine using the existing code. I totally agree with both of your comments, there is no way we can replicate MiPi spec into an different DT representation, this brings both maintenance overhead and is fragile. This a very big effort. Either we represent full ACPI into DT or use some know representation that linux understands and is maintained along with ACPI which is the C structure. > > However, that said if people want to work on doing so I am not > going to stand in the way. It does have the advantage it might > unblock things with the DT maintainers, since we can focus on > doing stuff in a more traditional DT style. This is really going to be a very very long shot, but as of today the need for this is slim, unless we see some platforms which have SDCA compliant codecs without a reference ACPI fw entires. > > I would also agree that this is reasonable as a half way house, > it seems like a good fallback mechanism to have for emergency > work arounds as well. If we can't reuse the existing parser then > there is a lot of work to define a new representation for DT. +1 > Ideally that would be worked through with the MIPI group as well > to avoid future specs causing catastrophic issues. But all that > will take significant time so we probably need an interim > solution. > --srini > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 9:09 ` Srinivas Kandagatla @ 2026-09-08 10:37 ` Richard Fitzgerald 2026-09-08 12:31 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Richard Fitzgerald @ 2026-09-08 10:37 UTC (permalink / raw) To: Srinivas Kandagatla, Charles Keepax Cc: Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 08/09/2026 10:09 am, Srinivas Kandagatla wrote: > On 9/8/26 9:49 AM, Charles Keepax wrote: >> On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: >>> On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: >>>> My take is that rather than encode all properties in C, it's probably >>>> worth exploring a DT representation of the concepts that *can* be used >>>> fairly easily and make the life of codec vendors easier - not as a> literal translation of ACPI. >>> >>> I agree a DT expression of the *concepts* not a literal DisCo >>> mirror would be a better A. I want to bring that back to LPC 2026 >>> DT MC as a concrete proposal grounded in the earlier DT-maintainer >>> feedback rather than re-litigating it inline on this series. Because >>> B stays put, switching A later is a change inside populate_function, >>> not an ABI break. so landing this series does not close the door on >>> the representation you're asking for. >> >> I am still not sure I really see what the value is in a >> completely different DT representation of the DisCo information, >> all it does is give us potential issues with future spec versions >> and create a whole bunch of new code than needs maintained. The >> ACPI representation translates perfectly well to DT, should work >> perfectly fine using the existing code. > > I totally agree with both of your comments, there is no way we can > replicate MiPi spec into an different DT representation, this brings > both maintenance overhead and is fragile. This a very big effort. But in your previous message in this thread you said the opposite: "I agree a DT expression of the *concepts* not a literal DisCo mirror would be a better A." ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 10:37 ` Richard Fitzgerald @ 2026-09-08 12:31 ` Srinivas Kandagatla 2026-09-08 13:20 ` Charles Keepax 0 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 12:31 UTC (permalink / raw) To: Richard Fitzgerald, Charles Keepax Cc: Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 11:37 AM, Richard Fitzgerald wrote: > On 08/09/2026 10:09 am, Srinivas Kandagatla wrote: >> On 9/8/26 9:49 AM, Charles Keepax wrote: >>> On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: >>>> On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: >>>>> My take is that rather than encode all properties in C, it's probably >>>>> worth exploring a DT representation of the concepts that *can* be used >>>>> fairly easily and make the life of codec vendors easier - not as a> >>>>> literal translation of ACPI. >>>> >>>> I agree a DT expression of the *concepts* not a literal DisCo >>>> mirror would be a better A. I want to bring that back to LPC 2026 >>>> DT MC as a concrete proposal grounded in the earlier DT-maintainer >>>> feedback rather than re-litigating it inline on this series. Because >>>> B stays put, switching A later is a change inside populate_function, >>>> not an ABI break. so landing this series does not close the door on >>>> the representation you're asking for. >>> >>> I am still not sure I really see what the value is in a >>> completely different DT representation of the DisCo information, >>> all it does is give us potential issues with future spec versions >>> and create a whole bunch of new code than needs maintained. The >>> ACPI representation translates perfectly well to DT, should work >>> perfectly fine using the existing code. >> >> I totally agree with both of your comments, there is no way we can >> replicate MiPi spec into an different DT representation, this brings >> both maintenance overhead and is fragile. This a very big effort. > > But in your previous message in this thread you said the opposite: > Yes I did say that, and I still think an intermediate DT representation would be a better A in principle. What I'm highlighting is the cost side, any intermediate representation, by definition, deviates from the original MIPI representation. That deviation has to be kept in sync with the SDCA spec, tracked across spec revisions, and translated back to struct sdca_function_data at runtime. It's a real overhead, not a blocker, just something to weigh against the "concepts read cleaner in DT" benefit. That's why the current series takes the lower-overhead path (mechanical transcription in C) rather than proposing an intermediate DT format as part of this submission. If the concept-DT direction gets traction at LPC 2026 DT MC, we can revisit. > "I agree a DT expression of the *concepts* not a literal DisCo > mirror would be a better A." --srini ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 12:31 ` Srinivas Kandagatla @ 2026-09-08 13:20 ` Charles Keepax 2026-09-08 13:34 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-08 13:20 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Richard Fitzgerald, Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Tue, Sep 08, 2026 at 01:31:37PM +0100, Srinivas Kandagatla wrote: > On 9/8/26 11:37 AM, Richard Fitzgerald wrote: > > On 08/09/2026 10:09 am, Srinivas Kandagatla wrote: > >> On 9/8/26 9:49 AM, Charles Keepax wrote: > >>> On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: > >>>> On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: > >>>>> My take is that rather than encode all properties in C, it's probably > >>>>> worth exploring a DT representation of the concepts that *can* be used > >>>>> fairly easily and make the life of codec vendors easier - not as a> > >>>>> literal translation of ACPI. > >>>> > >>>> I agree a DT expression of the *concepts* not a literal DisCo > >>>> mirror would be a better A. I want to bring that back to LPC 2026 > >>>> DT MC as a concrete proposal grounded in the earlier DT-maintainer > >>>> feedback rather than re-litigating it inline on this series. Because > >>>> B stays put, switching A later is a change inside populate_function, > >>>> not an ABI break. so landing this series does not close the door on > >>>> the representation you're asking for. > >>> > >>> I am still not sure I really see what the value is in a > >>> completely different DT representation of the DisCo information, > >>> all it does is give us potential issues with future spec versions > >>> and create a whole bunch of new code than needs maintained. The > >>> ACPI representation translates perfectly well to DT, should work > >>> perfectly fine using the existing code. > >> > >> I totally agree with both of your comments, there is no way we can > >> replicate MiPi spec into an different DT representation, this brings > >> both maintenance overhead and is fragile. This a very big effort. > > > > But in your previous message in this thread you said the opposite: > > Yes I did say that, and I still think an intermediate DT > representation would be a better A in principle. What I'm > highlighting is the cost side, any intermediate representation, by > definition, deviates from the original MIPI representation. That > deviation has to be kept in sync with the SDCA spec, tracked across > spec revisions, and translated back to struct sdca_function_data at > runtime. It's a real overhead, not a blocker, just something to > weigh against the "concepts read cleaner in DT" benefit. I am very sorry but this reads quite strangely and I am really struggling to understand your position. I think I am reading it as you are happy to go with the group concensus on how the DT is represented and don't favour either approach. > That's why the current series takes the lower-overhead path > (mechanical transcription in C) rather than proposing an intermediate > DT format as part of this submission. If the concept-DT direction > gets traction at LPC 2026 DT MC, we can revisit. This part I follow slightly better, I would be happy to proceed, pending reviews with the current approach, although if Pierre is or not probably remains to be seen. I would assume the DT guys would be happy with a completely different representation that was more idiomatic DT, that is essentially what they have already said. The question I was aiming for was less whether they will be on board with that and more if it is a good idea. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 13:20 ` Charles Keepax @ 2026-09-08 13:34 ` Srinivas Kandagatla 2026-09-08 14:22 ` Pierre-Louis Bossart 0 siblings, 1 reply; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 13:34 UTC (permalink / raw) To: Charles Keepax Cc: Richard Fitzgerald, Pierre-Louis Bossart, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 2:20 PM, Charles Keepax wrote: > On Tue, Sep 08, 2026 at 01:31:37PM +0100, Srinivas Kandagatla wrote: >> On 9/8/26 11:37 AM, Richard Fitzgerald wrote: >>> On 08/09/2026 10:09 am, Srinivas Kandagatla wrote: >>>> On 9/8/26 9:49 AM, Charles Keepax wrote: >>>>> On Mon, Sep 07, 2026 at 11:37:49PM +0100, Srinivas Kandagatla wrote: >>>>>> On 9/7/26 8:47 PM, Pierre-Louis Bossart wrote: >>>>>>> My take is that rather than encode all properties in C, it's probably >>>>>>> worth exploring a DT representation of the concepts that *can* be used >>>>>>> fairly easily and make the life of codec vendors easier - not as a> >>>>>>> literal translation of ACPI. >>>>>> >>>>>> I agree a DT expression of the *concepts* not a literal DisCo >>>>>> mirror would be a better A. I want to bring that back to LPC 2026 >>>>>> DT MC as a concrete proposal grounded in the earlier DT-maintainer >>>>>> feedback rather than re-litigating it inline on this series. Because >>>>>> B stays put, switching A later is a change inside populate_function, >>>>>> not an ABI break. so landing this series does not close the door on >>>>>> the representation you're asking for. >>>>> >>>>> I am still not sure I really see what the value is in a >>>>> completely different DT representation of the DisCo information, >>>>> all it does is give us potential issues with future spec versions >>>>> and create a whole bunch of new code than needs maintained. The >>>>> ACPI representation translates perfectly well to DT, should work >>>>> perfectly fine using the existing code. >>>> >>>> I totally agree with both of your comments, there is no way we can >>>> replicate MiPi spec into an different DT representation, this brings >>>> both maintenance overhead and is fragile. This a very big effort. >>> >>> But in your previous message in this thread you said the opposite: >> >> Yes I did say that, and I still think an intermediate DT >> representation would be a better A in principle. What I'm >> highlighting is the cost side, any intermediate representation, by >> definition, deviates from the original MIPI representation. That >> deviation has to be kept in sync with the SDCA spec, tracked across >> spec revisions, and translated back to struct sdca_function_data at >> runtime. It's a real overhead, not a blocker, just something to >> weigh against the "concepts read cleaner in DT" benefit. > > I am very sorry but this reads quite strangely and I am really > struggling to understand your position. I think I am reading it > as you are happy to go with the group concensus on how the DT is > represented and don't favour either approach. Apologies for not being clear. I do favor the C-style one that is submitted given the discussions that we had so far. You also seems to be ok with that approach, I will wait for Pierre to comment and stay with it. Am happy to fix any changes as required. > >> That's why the current series takes the lower-overhead path >> (mechanical transcription in C) rather than proposing an intermediate >> DT format as part of this submission. If the concept-DT direction >> gets traction at LPC 2026 DT MC, we can revisit. > > This part I follow slightly better, I would be happy to proceed, > pending reviews with the current approach, although if Pierre > is or not probably remains to be seen. @Pierre Please let us know if you are okay with this approach of C structures. --srini > > I would assume the DT guys would be happy with a completely > different representation that was more idiomatic DT, that is > essentially what they have already said. The question I was > aiming for was less whether they will be on board with that > and more if it is a good idea. > > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 13:34 ` Srinivas Kandagatla @ 2026-09-08 14:22 ` Pierre-Louis Bossart 2026-09-08 15:33 ` Charles Keepax 0 siblings, 1 reply; 45+ messages in thread From: Pierre-Louis Bossart @ 2026-09-08 14:22 UTC (permalink / raw) To: Srinivas Kandagatla, Charles Keepax Cc: Richard Fitzgerald, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel >>> That's why the current series takes the lower-overhead path >>> (mechanical transcription in C) rather than proposing an intermediate >>> DT format as part of this submission. If the concept-DT direction >>> gets traction at LPC 2026 DT MC, we can revisit. >> >> This part I follow slightly better, I would be happy to proceed, >> pending reviews with the current approach, although if Pierre >> is or not probably remains to be seen. > > @Pierre > Please let us know if you are okay with this approach of C structures. Now that I have more context, I don't have any objections with Srini's proposal. It'd a good step forward to use common class drivers across multiple vendors, we'll probably find a couple of bugs or harden the SDCA core and that'd be good progress for everyone developing or depending on the ASoC/SoundWire/SDCA frameworks. If this means that in an intermediate step C tables are used, that's fine with me. In the long run, things might change with additional options such as: a) ACPI support on all platforms b) DT hybrid mode to reuse ACPI/DSDT tables c) DT native representation of properties. We can review the preferred direction when the platform firmware plumbing improves. I believe the point about binary/opaque data was noted by Srini, it can be handled in multiple ways and there's no reason to block. Best to start small at the SDCA level with incremental changes later on how to fetch DisCo information from platform firmware - that wouldn't change the system behavior, only optimize by making the translation from DSDT to C tables un-necessary. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 14:22 ` Pierre-Louis Bossart @ 2026-09-08 15:33 ` Charles Keepax 2026-09-08 15:34 ` Srinivas Kandagatla 0 siblings, 1 reply; 45+ messages in thread From: Charles Keepax @ 2026-09-08 15:33 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, Richard Fitzgerald, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Tue, Sep 08, 2026 at 04:22:53PM +0200, Pierre-Louis Bossart wrote: > >>> That's why the current series takes the lower-overhead path > >>> (mechanical transcription in C) rather than proposing an intermediate > >>> DT format as part of this submission. If the concept-DT direction > >>> gets traction at LPC 2026 DT MC, we can revisit. > >> > >> This part I follow slightly better, I would be happy to proceed, > >> pending reviews with the current approach, although if Pierre > >> is or not probably remains to be seen. > > > > @Pierre > > Please let us know if you are okay with this approach of C structures. > Now that I have more context, I don't have any objections with Srini's > proposal. It'd a good step forward to use common class drivers across > multiple vendors, we'll probably find a couple of bugs or harden the > SDCA core and that'd be good progress for everyone developing or > depending on the ASoC/SoundWire/SDCA frameworks. > > If this means that in an intermediate step C tables are used, that's > fine with me. In the long run, things might change with additional > options such as: > a) ACPI support on all platforms > b) DT hybrid mode to reuse ACPI/DSDT tables > c) DT native representation of properties. > We can review the preferred direction when the platform firmware > plumbing improves. > > I believe the point about binary/opaque data was noted by Srini, it can > be handled in multiple ways and there's no reason to block. > > Best to start small at the SDCA level with incremental changes later on > how to fetch DisCo information from platform firmware - that wouldn't > change the system behavior, only optimize by making the translation from > DSDT to C tables un-necessary. Super, thanks Pierre. I will start working through a more detailed review of the series. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-08 15:33 ` Charles Keepax @ 2026-09-08 15:34 ` Srinivas Kandagatla 0 siblings, 0 replies; 45+ messages in thread From: Srinivas Kandagatla @ 2026-09-08 15:34 UTC (permalink / raw) To: Charles Keepax, Pierre-Louis Bossart Cc: Richard Fitzgerald, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On 9/8/26 4:33 PM, Charles Keepax wrote: > On Tue, Sep 08, 2026 at 04:22:53PM +0200, Pierre-Louis Bossart wrote: >>>>> That's why the current series takes the lower-overhead path >>>>> (mechanical transcription in C) rather than proposing an intermediate >>>>> DT format as part of this submission. If the concept-DT direction >>>>> gets traction at LPC 2026 DT MC, we can revisit. >>>> >>>> This part I follow slightly better, I would be happy to proceed, >>>> pending reviews with the current approach, although if Pierre >>>> is or not probably remains to be seen. >>> >>> @Pierre >>> Please let us know if you are okay with this approach of C structures. >> Now that I have more context, I don't have any objections with Srini's >> proposal. It'd a good step forward to use common class drivers across >> multiple vendors, we'll probably find a couple of bugs or harden the >> SDCA core and that'd be good progress for everyone developing or >> depending on the ASoC/SoundWire/SDCA frameworks. >> >> If this means that in an intermediate step C tables are used, that's >> fine with me. In the long run, things might change with additional >> options such as: >> a) ACPI support on all platforms >> b) DT hybrid mode to reuse ACPI/DSDT tables >> c) DT native representation of properties. >> We can review the preferred direction when the platform firmware >> plumbing improves. >> >> I believe the point about binary/opaque data was noted by Srini, it can >> be handled in multiple ways and there's no reason to block. >> >> Best to start small at the SDCA level with incremental changes later on >> how to fetch DisCo information from platform firmware - that wouldn't >> change the system behavior, only optimize by making the translation from >> DSDT to C tables un-necessary. > > Super, thanks Pierre. I will start working through a more > detailed review of the series. Thanks Charles and Pierre for reviewing this series. --srini > > Thanks, > Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart @ 2026-09-08 15:58 ` Uwe Kleine-König 2026-09-08 16:20 ` Charles Keepax 2 siblings, 0 replies; 45+ messages in thread From: Uwe Kleine-König @ 2026-09-08 15:58 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Mark Brown, Rob Herring, Charles Keepax, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1327 bytes --] Hello, On Mon, Sep 07, 2026 at 09:37:25AM +0100, Srinivas Kandagatla wrote: > +#include <linux/mod_devicetable.h> Please don't include <linux/mod_devicetable.h>. Either rely on <linux/soundwire/sdw.h> to provide sdw_device_id (my preferred way), or (if you want full iwyu) include <linux/device-id/sdw.h>. > [...] > +static const struct sdw_device_id wcd9378_sdw_id[] = { > + SDW_SLAVE_ENTRY(0x0217, 0x0110, 0), > + {} > +}; s/{}/{ }/ please to match the common style. > +MODULE_DEVICE_TABLE(sdw, wcd9378_sdw_id); > + > +static struct sdw_driver wcd9378_sdw_driver = { > + .driver = { > + .name = "wcd9378", > + .pm = pm_ptr(&wcd9378_sdw_pm_ops), > + }, > + .probe = wcd9378_sdca_probe, > + .remove = wcd9378_sdca_remove, > + .id_table = wcd9378_sdw_id, > + .ops = &wcd9378_sdw_ops, > +}; subjective feedback: I'm not a fan of aligning the = in such initialisations. They tend to get inconsistent over time, and sometimes (like here) they even start inconsistently. So if you ask me, use a single space before each =. > +module_sdw_driver(wcd9378_sdw_driver); > + > +MODULE_DESCRIPTION("Qualcomm WCD9378 (Tambora) SoundWire codec"); > +MODULE_AUTHOR("Qualcomm Technologies, Inc."); > +MODULE_LICENSE("Dual BSD/GPL"); > +MODULE_IMPORT_NS("SND_SOC_SDCA"); > +MODULE_IMPORT_NS("SND_SOC_SDCA_CLASS"); Best regards Uwe [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-08 15:58 ` Uwe Kleine-König @ 2026-09-08 16:20 ` Charles Keepax 2 siblings, 0 replies; 45+ messages in thread From: Charles Keepax @ 2026-09-08 16:20 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bard Liao, Jaroslav Kysela, Liam Girdwood, Maciej Strozek, Pierre-Louis Bossart, Takashi Iwai, Faiz Nabi Kuchay, Jorijn van der Graaf, patches, linux-sound, devicetree, linux-arm-msm, linux-kernel On Mon, Sep 07, 2026 at 09:37:25AM +0100, Srinivas Kandagatla wrote: > Add support for the Qualcomm Tambora (WCD9378) headset codec in SDCA > mode over SoundWire. On ARM/DT platforms without ACPI/DisCo firmware > the SDCA topology and SoundWire port properties are supplied as static > data through the codec driver. > > The codec exposes a single SimpleJack SDCA Function providing: > - Headphone playback via FU 6 (mute + Q7.8 volume) and OT 43/45. > - Headset mic capture via IT 33 with MICB2 bias derived from DT > (qcom,micbias2-microvolt). > - MBHC-based headset jack detection. > > Implements: > - sdw_slave_ops.read_prop: SoundWire slave properties and dpn caps > for the compute-mode dataports. > - sdca_class_hw_ops.hw_init: enables supplies, toggles the reset > GPIO, and enables the vendor TX PDM clock via SCP. > - sdca_class_hw_ops.populate_function: fills the SDCA Function data > (entities, clusters, init_table) from static tables and patches > the IT 33 MIC_BIAS default with the DT-derived per-slave value. > > Binds SoundWire slave id 0x0217:0x0110 when qcom,compute-mode is set > on the DT node. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > --- > +int wcd9378_sdca_read_prop(struct sdw_slave *slave) > +{ > + struct sdw_slave_prop *prop = &slave->prop; > + struct device *dev = &slave->dev; > + struct sdw_dpn_prop *sink, *src; > + int ret; > + > + ret = sdca_class_read_prop(slave); > + if (ret) > + return ret; I am not sure it is worth calling this. All this does that doesn't come from the firmware is set use_domain_irq and scp_int1_mask, I would just do both of those locally, calling this means you technically process a large number of DT properties that aren't in your binding doc and also you devm allocate a bunch of things you won't use. > + /* Compute-mode fixed SoundWire slave properties (not described in DT) */ > + prop->simple_clk_stop_capable = true; > + prop->paging_support = true; > + prop->clock_reg_supported = true; > + prop->lane_control_support = true; > + > + /* Source ports: DP2 (headset mic), DP5 (optimisation TX). */ > + prop->source_ports = BIT(2) | BIT(5); > + /* Sink ports: DP6 (HPH audio), DP7 (HPH envelope), DP8 (optimisation RX). */ > + prop->sink_ports = BIT(6) | BIT(7) | BIT(8); > + > + src = devm_kcalloc(dev, 2, sizeof(*src), GFP_KERNEL); > + if (!src) > + return -ENOMEM; > + > + src[0].num = 2; > + src[0].type = SDW_DPN_SIMPLE; > + src[0].simple_ch_prep_sm = true; > + src[0].ch_prep_timeout = 10; > + src[0].max_ch = 1; > + src[0].min_ch = 1; > + > + src[1].num = 5; > + src[1].type = SDW_DPN_SIMPLE; > + src[1].simple_ch_prep_sm = true; > + src[1].ch_prep_timeout = 10; > + src[1].max_ch = 1; > + src[1].min_ch = 1; Minor nit: Probably simpler to have a static array and kmemdup it. > +static int wcd9378_sdca_populate_function(struct sdw_slave *slave, > + struct sdca_function_data *function) > +{ > + /* @function->desc is already set by the framework; fill payload only. */ > + if (function->desc->type != wcd9378_sdca_desc.type) > + return -EINVAL; > + > + function->num_entities = wcd9378_sdca_data.num_entities; > + function->entities = wcd9378_sdca_data.entities; > + function->num_clusters = wcd9378_sdca_data.num_clusters; > + function->clusters = wcd9378_sdca_data.clusters; > + function->num_init_table = wcd9378_sdca_data.num_init_table; > + function->init_table = wcd9378_sdca_data.init_table; > + function->reset_max_delay = wcd9378_sdca_data.reset_max_delay; > + > + /* Elevate is_volatile / has_reset to match the DisCo/ACPI path. */ > + sdca_apply_default_control_classifiers(function); Just factor those into the static tables, if we have a bunch of static data might as well just fill it all in. Doing it this way is confusing because the static data is incorrect then fixed up later. Thanks, Charles ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2026-09-09 8:35 UTC | newest] Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-07 8:37 [PATCH v2 00/11] ASoC: SDCA: enable on DT platforms and add Qualcomm WCD9378 (Tambora) codec Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 01/11] ASoC: SDCA: allow building without ACPI Srinivas Kandagatla 2026-09-07 8:54 ` Richard Fitzgerald 2026-09-07 9:09 ` Takashi Iwai 2026-09-07 8:37 ` [PATCH v2 02/11] ASoC: SDCA: export PM helpers keyed on sdca_class_drv Srinivas Kandagatla 2026-09-08 16:22 ` Charles Keepax 2026-09-08 17:49 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library Srinivas Kandagatla 2026-09-07 11:31 ` Pierre-Louis Bossart 2026-09-07 13:29 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 04/11] ASoC: SDCA: add hw_ops with hw_init hook Srinivas Kandagatla 2026-09-07 11:29 ` Pierre-Louis Bossart 2026-09-07 13:33 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data Srinivas Kandagatla 2026-09-07 11:28 ` Pierre-Louis Bossart 2026-09-07 13:16 ` Charles Keepax 2026-09-08 16:25 ` Charles Keepax 2026-09-08 18:00 ` Srinivas Kandagatla 2026-09-09 8:34 ` Charles Keepax 2026-09-07 8:37 ` [PATCH v2 06/11] ASoC: SDCA: class_function: xlate sound-dai cell by entity index Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 07/11] ASoC: SDCA: register SDCA_FUNCTION_TYPE_SIMPLE_JACK in class function driver Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 8:37 ` [PATCH v2 08/11] ASoC: SDCA: make find_sdca_control_reset() return void Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 13:03 ` Charles Keepax 2026-09-07 13:16 ` Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 09/11] ASoC: SDCA: add sdca_apply_default_control_classifiers() helper Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 10/11] dt-bindings: sound: qcom: add Tambora WCD9378 SDCA codec Srinivas Kandagatla 2026-09-07 8:37 ` [PATCH v2 11/11] ASoC: codecs: add Qualcomm Tambora (WCD9378) " Srinivas Kandagatla 2026-09-07 11:32 ` Pierre-Louis Bossart 2026-09-07 13:03 ` Srinivas Kandagatla 2026-09-07 19:47 ` Pierre-Louis Bossart 2026-09-07 21:26 ` Mark Brown 2026-09-07 22:37 ` Srinivas Kandagatla 2026-09-08 8:49 ` Charles Keepax 2026-09-08 9:09 ` Srinivas Kandagatla 2026-09-08 10:37 ` Richard Fitzgerald 2026-09-08 12:31 ` Srinivas Kandagatla 2026-09-08 13:20 ` Charles Keepax 2026-09-08 13:34 ` Srinivas Kandagatla 2026-09-08 14:22 ` Pierre-Louis Bossart 2026-09-08 15:33 ` Charles Keepax 2026-09-08 15:34 ` Srinivas Kandagatla 2026-09-08 15:58 ` Uwe Kleine-König 2026-09-08 16:20 ` Charles Keepax
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®