* [PATCH v3 RESEND 01/19] ASoC: SDCA: Rename SoundWire struct device variables
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 02/19] regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave Charles Keepax
` (19 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
The SDCA core processes two different struct device's at various times,
usually it is processing the struct device associated with an individual
SDCA Function driver. However, there are times that it is processing the
struct device associated with the actual SoundWire peripheral, usually
the parent of the Function device.
To ensure this distinction remains clear in the code, rename the variable
when handling the actual SoundWire device to sdev.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
sound/soc/sdca/sdca_functions.c | 6 +++---
sound/soc/sdca/sdca_interrupts.c | 14 +++++++-------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 13f68f7b6dd6a..5f76ff4345ff5 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -179,11 +179,11 @@ static int find_sdca_function(struct acpi_device *adev, void *data)
*/
void sdca_lookup_functions(struct sdw_slave *slave)
{
- struct device *dev = &slave->dev;
- struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+ struct device *sdev = &slave->dev;
+ struct acpi_device *adev = to_acpi_device_node(sdev->fwnode);
if (!adev) {
- dev_info(dev, "no matching ACPI device found, ignoring peripheral\n");
+ dev_info(sdev, "no matching ACPI device found, ignoring peripheral\n");
return;
}
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 79bf3042f57d4..1926c0846846f 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -400,7 +400,7 @@ EXPORT_SYMBOL_NS_GPL(sdca_irq_populate, "SND_SOC_SDCA");
/**
* sdca_irq_allocate - allocate an SDCA interrupt structure for a device
- * @dev: Device pointer against which things should be allocated.
+ * @sdev: Device pointer against which things should be allocated.
* @regmap: regmap to be used for accessing the SDCA IRQ registers.
* @irq: The interrupt number.
*
@@ -411,30 +411,30 @@ EXPORT_SYMBOL_NS_GPL(sdca_irq_populate, "SND_SOC_SDCA");
* Return: A pointer to the allocated sdca_interrupt_info struct, or an
* error code.
*/
-struct sdca_interrupt_info *sdca_irq_allocate(struct device *dev,
+struct sdca_interrupt_info *sdca_irq_allocate(struct device *sdev,
struct regmap *regmap, int irq)
{
struct sdca_interrupt_info *info;
int ret;
- info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+ info = devm_kzalloc(sdev, sizeof(*info), GFP_KERNEL);
if (!info)
return ERR_PTR(-ENOMEM);
info->irq_chip = sdca_irq_chip;
- ret = devm_mutex_init(dev, &info->irq_lock);
+ ret = devm_mutex_init(sdev, &info->irq_lock);
if (ret)
return ERR_PTR(ret);
- ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT, 0,
+ ret = devm_regmap_add_irq_chip(sdev, regmap, irq, IRQF_ONESHOT, 0,
&info->irq_chip, &info->irq_data);
if (ret) {
- dev_err(dev, "failed to register irq chip: %d\n", ret);
+ dev_err(sdev, "failed to register irq chip: %d\n", ret);
return ERR_PTR(ret);
}
- dev_dbg(dev, "registered on irq %d\n", irq);
+ dev_dbg(sdev, "registered on irq %d\n", irq);
return info;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 02/19] regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 01/19] ASoC: SDCA: Rename SoundWire struct device variables Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 03/19] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers Charles Keepax
` (18 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Currently, the code assumes that the device that registered the
MBQ register map is the actual SoundWire slave device. This works
fine for all current users, however future SDCA devices will
likely be implemented with the SoundWire slave as a parent device
and separate child drivers with regmaps for each audio Function.
Update the regmap_init_sdw_mbq_cfg macro to allow these two
to be specified separately.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
drivers/base/regmap/regmap-sdw-mbq.c | 23 ++++++++++++-----------
include/linux/regmap.h | 21 +++++++++++----------
sound/soc/codecs/rt722-sdca-sdw.c | 4 +++-
3 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw-mbq.c b/drivers/base/regmap/regmap-sdw-mbq.c
index 86644bbd07100..8b7d34a6080d2 100644
--- a/drivers/base/regmap/regmap-sdw-mbq.c
+++ b/drivers/base/regmap/regmap-sdw-mbq.c
@@ -15,6 +15,7 @@
struct regmap_mbq_context {
struct device *dev;
+ struct sdw_slave *sdw;
struct regmap_sdw_mbq_cfg cfg;
@@ -46,7 +47,7 @@ static bool regmap_sdw_mbq_deferrable(struct regmap_mbq_context *ctx, unsigned i
static int regmap_sdw_mbq_poll_busy(struct sdw_slave *slave, unsigned int reg,
struct regmap_mbq_context *ctx)
{
- struct device *dev = &slave->dev;
+ struct device *dev = ctx->dev;
int val, ret = 0;
dev_dbg(dev, "Deferring transaction for 0x%x\n", reg);
@@ -96,8 +97,7 @@ static int regmap_sdw_mbq_write_impl(struct sdw_slave *slave,
static int regmap_sdw_mbq_write(void *context, unsigned int reg, unsigned int val)
{
struct regmap_mbq_context *ctx = context;
- struct device *dev = ctx->dev;
- struct sdw_slave *slave = dev_to_sdw_dev(dev);
+ struct sdw_slave *slave = ctx->sdw;
bool deferrable = regmap_sdw_mbq_deferrable(ctx, reg);
int mbq_size = regmap_sdw_mbq_size(ctx, reg);
int ret;
@@ -156,8 +156,7 @@ static int regmap_sdw_mbq_read_impl(struct sdw_slave *slave,
static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *val)
{
struct regmap_mbq_context *ctx = context;
- struct device *dev = ctx->dev;
- struct sdw_slave *slave = dev_to_sdw_dev(dev);
+ struct sdw_slave *slave = ctx->sdw;
bool deferrable = regmap_sdw_mbq_deferrable(ctx, reg);
int mbq_size = regmap_sdw_mbq_size(ctx, reg);
int ret;
@@ -208,6 +207,7 @@ static int regmap_sdw_mbq_config_check(const struct regmap_config *config)
static struct regmap_mbq_context *
regmap_sdw_mbq_gen_context(struct device *dev,
+ struct sdw_slave *sdw,
const struct regmap_config *config,
const struct regmap_sdw_mbq_cfg *mbq_config)
{
@@ -218,6 +218,7 @@ regmap_sdw_mbq_gen_context(struct device *dev,
return ERR_PTR(-ENOMEM);
ctx->dev = dev;
+ ctx->sdw = sdw;
if (mbq_config)
ctx->cfg = *mbq_config;
@@ -228,7 +229,7 @@ regmap_sdw_mbq_gen_context(struct device *dev,
return ctx;
}
-struct regmap *__regmap_init_sdw_mbq(struct sdw_slave *sdw,
+struct regmap *__regmap_init_sdw_mbq(struct device *dev, struct sdw_slave *sdw,
const struct regmap_config *config,
const struct regmap_sdw_mbq_cfg *mbq_config,
struct lock_class_key *lock_key,
@@ -241,16 +242,16 @@ struct regmap *__regmap_init_sdw_mbq(struct sdw_slave *sdw,
if (ret)
return ERR_PTR(ret);
- ctx = regmap_sdw_mbq_gen_context(&sdw->dev, config, mbq_config);
+ ctx = regmap_sdw_mbq_gen_context(dev, sdw, config, mbq_config);
if (IS_ERR(ctx))
return ERR_CAST(ctx);
- return __regmap_init(&sdw->dev, ®map_sdw_mbq, ctx,
+ return __regmap_init(dev, ®map_sdw_mbq, ctx,
config, lock_key, lock_name);
}
EXPORT_SYMBOL_GPL(__regmap_init_sdw_mbq);
-struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave *sdw,
+struct regmap *__devm_regmap_init_sdw_mbq(struct device *dev, struct sdw_slave *sdw,
const struct regmap_config *config,
const struct regmap_sdw_mbq_cfg *mbq_config,
struct lock_class_key *lock_key,
@@ -263,11 +264,11 @@ struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave *sdw,
if (ret)
return ERR_PTR(ret);
- ctx = regmap_sdw_mbq_gen_context(&sdw->dev, config, mbq_config);
+ ctx = regmap_sdw_mbq_gen_context(dev, sdw, config, mbq_config);
if (IS_ERR(ctx))
return ERR_CAST(ctx);
- return __devm_regmap_init(&sdw->dev, ®map_sdw_mbq, ctx,
+ return __devm_regmap_init(dev, ®map_sdw_mbq, ctx,
config, lock_key, lock_name);
}
EXPORT_SYMBOL_GPL(__devm_regmap_init_sdw_mbq);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 4e1ac1fbcec43..70daec535976d 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -676,7 +676,7 @@ struct regmap *__regmap_init_sdw(struct sdw_slave *sdw,
const struct regmap_config *config,
struct lock_class_key *lock_key,
const char *lock_name);
-struct regmap *__regmap_init_sdw_mbq(struct sdw_slave *sdw,
+struct regmap *__regmap_init_sdw_mbq(struct device *dev, struct sdw_slave *sdw,
const struct regmap_config *config,
const struct regmap_sdw_mbq_cfg *mbq_config,
struct lock_class_key *lock_key,
@@ -738,7 +738,7 @@ struct regmap *__devm_regmap_init_sdw(struct sdw_slave *sdw,
const struct regmap_config *config,
struct lock_class_key *lock_key,
const char *lock_name);
-struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave *sdw,
+struct regmap *__devm_regmap_init_sdw_mbq(struct device *dev, struct sdw_slave *sdw,
const struct regmap_config *config,
const struct regmap_sdw_mbq_cfg *mbq_config,
struct lock_class_key *lock_key,
@@ -970,7 +970,7 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
*/
#define regmap_init_sdw_mbq(sdw, config) \
__regmap_lockdep_wrapper(__regmap_init_sdw_mbq, #config, \
- sdw, config, NULL)
+ &sdw->dev, sdw, config, NULL)
/**
* regmap_init_sdw_mbq_cfg() - Initialise MBQ SDW register map with config
@@ -983,9 +983,9 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
* to a struct regmap. The regmap will be automatically freed by the
* device management code.
*/
-#define regmap_init_sdw_mbq_cfg(sdw, config, mbq_config) \
+#define regmap_init_sdw_mbq_cfg(dev, sdw, config, mbq_config) \
__regmap_lockdep_wrapper(__regmap_init_sdw_mbq, #config, \
- sdw, config, mbq_config)
+ dev, sdw, config, mbq_config)
/**
* regmap_init_spi_avmm() - Initialize register map for Intel SPI Slave
@@ -1198,12 +1198,13 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
*/
#define devm_regmap_init_sdw_mbq(sdw, config) \
__regmap_lockdep_wrapper(__devm_regmap_init_sdw_mbq, #config, \
- sdw, config, NULL)
+ &sdw->dev, sdw, config, NULL)
/**
* devm_regmap_init_sdw_mbq_cfg() - Initialise managed MBQ SDW register map with config
*
- * @sdw: Device that will be interacted with
+ * @dev: Device that will be interacted with
+ * @sdw: SoundWire Device that will be interacted with
* @config: Configuration for register map
* @mbq_config: Properties for the MBQ registers
*
@@ -1211,9 +1212,9 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
* to a struct regmap. The regmap will be automatically freed by the
* device management code.
*/
-#define devm_regmap_init_sdw_mbq_cfg(sdw, config, mbq_config) \
- __regmap_lockdep_wrapper(__devm_regmap_init_sdw_mbq, \
- #config, sdw, config, mbq_config)
+#define devm_regmap_init_sdw_mbq_cfg(dev, sdw, config, mbq_config) \
+ __regmap_lockdep_wrapper(__devm_regmap_init_sdw_mbq, \
+ #config, dev, sdw, config, mbq_config)
/**
* devm_regmap_init_slimbus() - Initialise managed register map
diff --git a/sound/soc/codecs/rt722-sdca-sdw.c b/sound/soc/codecs/rt722-sdca-sdw.c
index 5ea40c1b159a8..a0f5601a262aa 100644
--- a/sound/soc/codecs/rt722-sdca-sdw.c
+++ b/sound/soc/codecs/rt722-sdca-sdw.c
@@ -419,7 +419,9 @@ static int rt722_sdca_sdw_probe(struct sdw_slave *slave,
struct regmap *regmap;
/* Regmap Initialization */
- regmap = devm_regmap_init_sdw_mbq_cfg(slave, &rt722_sdca_regmap, &rt722_mbq_config);
+ regmap = devm_regmap_init_sdw_mbq_cfg(&slave->dev, slave,
+ &rt722_sdca_regmap,
+ &rt722_mbq_config);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 03/19] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 01/19] ASoC: SDCA: Rename SoundWire struct device variables Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 02/19] regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 04/19] ASoC: SDCA: Pass SoundWire slave to HID Charles Keepax
` (17 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
SDCA interrupt handling is a bit odd, the SDCA IRQ registers are
defined on a device level but the handling of the IRQ is at the
Function level. As such the regmap IRQ's PM runtime operations need to
be on the device itself to ensure those registers are available but an
additional runtime get is required for the Function child when the IRQ
is actually handled. Add the required manual PM runtime gets.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
sound/soc/sdca/sdca_interrupts.c | 42 ++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 1926c0846846f..9295d283be910 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -11,7 +11,9 @@
#include <linux/bits.h>
#include <linux/cleanup.h>
#include <linux/device.h>
+#include <linux/dev_printk.h>
#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/soundwire/sdw.h>
#include <linux/soundwire/sdw_registers.h>
@@ -86,18 +88,25 @@ static irqreturn_t function_status_handler(int irq, void *data)
{
struct sdca_interrupt *interrupt = data;
struct device *dev = interrupt->component->dev;
+ irqreturn_t irqret = IRQ_NONE;
unsigned int reg, val;
unsigned long status;
unsigned int mask;
int ret;
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to resume for function status: %d\n", ret);
+ goto error;
+ }
+
reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
interrupt->control->sel, 0);
ret = regmap_read(interrupt->component->regmap, reg, &val);
if (ret < 0) {
dev_err(dev, "failed to read function status: %d\n", ret);
- return IRQ_NONE;
+ goto error;
}
dev_dbg(dev, "function status: %#x\n", val);
@@ -130,10 +139,13 @@ static irqreturn_t function_status_handler(int irq, void *data)
ret = regmap_write(interrupt->component->regmap, reg, val);
if (ret < 0) {
dev_err(dev, "failed to clear function status: %d\n", ret);
- return IRQ_NONE;
+ goto error;
}
- return IRQ_HANDLED;
+ irqret = IRQ_HANDLED;
+error:
+ pm_runtime_put(dev);
+ return irqret;
}
static irqreturn_t detected_mode_handler(int irq, void *data)
@@ -146,21 +158,28 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
struct snd_kcontrol *kctl = interrupt->priv;
struct snd_ctl_elem_value *ucontrol __free(kfree) = NULL;
struct soc_enum *soc_enum;
+ irqreturn_t irqret = IRQ_NONE;
unsigned int reg, val;
int ret;
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to resume for detected mode: %d\n", ret);
+ goto error;
+ }
+
if (!kctl) {
const char *name __free(kfree) = kasprintf(GFP_KERNEL, "%s %s",
interrupt->entity->label,
SDCA_CTL_SELECTED_MODE_NAME);
if (!name)
- return IRQ_NONE;
+ goto error;
kctl = snd_soc_component_get_kcontrol(component, name);
if (!kctl) {
dev_dbg(dev, "control not found: %s\n", name);
- return IRQ_NONE;
+ goto error;
}
interrupt->priv = kctl;
@@ -174,7 +193,7 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
ret = regmap_read(component->regmap, reg, &val);
if (ret < 0) {
dev_err(dev, "failed to read detected mode: %d\n", ret);
- return IRQ_NONE;
+ goto error;
}
switch (val) {
@@ -195,7 +214,7 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
ret = regmap_read(component->regmap, reg, &val);
if (ret) {
dev_err(dev, "failed to re-check selected mode: %d\n", ret);
- return IRQ_NONE;
+ goto error;
}
break;
default:
@@ -206,7 +225,7 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL);
if (!ucontrol)
- return IRQ_NONE;
+ goto error;
ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(soc_enum, val);
@@ -215,12 +234,15 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
up_write(rwsem);
if (ret < 0) {
dev_err(dev, "failed to update selected mode: %d\n", ret);
- return IRQ_NONE;
+ goto error;
}
snd_ctl_notify(card->snd_card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
- return IRQ_HANDLED;
+ irqret = IRQ_HANDLED;
+error:
+ pm_runtime_put(dev);
+ return irqret;
}
static int sdca_irq_request_locked(struct device *dev,
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 04/19] ASoC: SDCA: Pass SoundWire slave to HID
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (2 preceding siblings ...)
2025-10-20 15:54 ` [PATCH v3 RESEND 03/19] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 05/19] ASoC: SDCA: Pass device register map from IRQ alloc to handlers Charles Keepax
` (16 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
The SDCA HID code can't assume that the struct device it is passed is
the SoundWire slave device. HID is represented by a Function in SDCA and
will thus likely be implemented by a child driver. Update the code to
explicitly pass in the SoundWire slave device.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_function.h | 2 +-
include/sound/sdca_hid.h | 8 ++++++--
sound/soc/sdca/sdca_functions.c | 20 ++++++++++++--------
sound/soc/sdca/sdca_hid.c | 12 +++++-------
4 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index ea68856e4c8c4..51e12fcfc53cd 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -1332,7 +1332,7 @@ static inline u32 sdca_range_search(struct sdca_control_range *range,
return 0;
}
-int sdca_parse_function(struct device *dev,
+int sdca_parse_function(struct device *dev, struct sdw_slave *sdw,
struct sdca_function_desc *desc,
struct sdca_function_data *function);
diff --git a/include/sound/sdca_hid.h b/include/sound/sdca_hid.h
index 8ab3e498884ef..3a155835e035e 100644
--- a/include/sound/sdca_hid.h
+++ b/include/sound/sdca_hid.h
@@ -12,10 +12,14 @@
#include <linux/hid.h>
#if IS_ENABLED(CONFIG_SND_SOC_SDCA_HID)
-int sdca_add_hid_device(struct device *dev, struct sdca_entity *entity);
+
+int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
+ struct sdca_entity *entity);
#else
-static inline int sdca_add_hid_device(struct device *dev, struct sdca_entity *entity)
+
+static inline int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
+ struct sdca_entity *entity)
{
return 0;
}
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 5f76ff4345ff5..2d5d20e23e3c4 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -1253,7 +1253,8 @@ static int find_sdca_entity_ge(struct device *dev,
}
static int
-find_sdca_entity_hide(struct device *dev, struct fwnode_handle *function_node,
+find_sdca_entity_hide(struct device *dev, struct sdw_slave *sdw,
+ struct fwnode_handle *function_node,
struct fwnode_handle *entity_node, struct sdca_entity *entity)
{
struct sdca_entity_hide *hide = &entity->hide;
@@ -1328,7 +1329,7 @@ find_sdca_entity_hide(struct device *dev, struct fwnode_handle *function_node,
report_desc, nval);
/* add HID device */
- ret = sdca_add_hid_device(dev, entity);
+ ret = sdca_add_hid_device(dev, sdw, entity);
if (ret) {
dev_err(dev, "%pfwP: failed to add HID device: %d\n", entity_node, ret);
return ret;
@@ -1339,7 +1340,7 @@ find_sdca_entity_hide(struct device *dev, struct fwnode_handle *function_node,
return 0;
}
-static int find_sdca_entity(struct device *dev,
+static int find_sdca_entity(struct device *dev, struct sdw_slave *sdw,
struct fwnode_handle *function_node,
struct fwnode_handle *entity_node,
struct sdca_entity *entity)
@@ -1381,7 +1382,8 @@ static int find_sdca_entity(struct device *dev,
ret = find_sdca_entity_ge(dev, entity_node, entity);
break;
case SDCA_ENTITY_TYPE_HIDE:
- ret = find_sdca_entity_hide(dev, function_node, entity_node, entity);
+ ret = find_sdca_entity_hide(dev, sdw, function_node,
+ entity_node, entity);
break;
default:
break;
@@ -1396,7 +1398,7 @@ static int find_sdca_entity(struct device *dev,
return 0;
}
-static int find_sdca_entities(struct device *dev,
+static int find_sdca_entities(struct device *dev, struct sdw_slave *sdw,
struct fwnode_handle *function_node,
struct sdca_function_data *function)
{
@@ -1448,7 +1450,8 @@ static int find_sdca_entities(struct device *dev,
return -EINVAL;
}
- ret = find_sdca_entity(dev, function_node, entity_node, &entities[i]);
+ ret = find_sdca_entity(dev, sdw, function_node,
+ entity_node, &entities[i]);
fwnode_handle_put(entity_node);
if (ret)
return ret;
@@ -1927,12 +1930,13 @@ static int find_sdca_clusters(struct device *dev,
/**
* sdca_parse_function - parse ACPI DisCo for a Function
* @dev: Pointer to device against which function data will be allocated.
+ * @sdw: SoundWire slave device to be processed.
* @function_desc: Pointer to the Function short descriptor.
* @function: Pointer to the Function information, to be populated.
*
* Return: Returns 0 for success.
*/
-int sdca_parse_function(struct device *dev,
+int sdca_parse_function(struct device *dev, struct sdw_slave *sdw,
struct sdca_function_desc *function_desc,
struct sdca_function_data *function)
{
@@ -1953,7 +1957,7 @@ int sdca_parse_function(struct device *dev,
if (ret)
return ret;
- ret = find_sdca_entities(dev, function_desc->node, function);
+ ret = find_sdca_entities(dev, sdw, function_desc->node, function);
if (ret)
return ret;
diff --git a/sound/soc/sdca/sdca_hid.c b/sound/soc/sdca/sdca_hid.c
index 967f7ec6fb79d..53dad1a524d4b 100644
--- a/sound/soc/sdca/sdca_hid.c
+++ b/sound/soc/sdca/sdca_hid.c
@@ -82,15 +82,13 @@ static const struct hid_ll_driver sdw_hid_driver = {
.raw_request = sdwhid_raw_request,
};
-int sdca_add_hid_device(struct device *dev, struct sdca_entity *entity)
+int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
+ struct sdca_entity *entity)
{
- struct sdw_bus *bus;
+ struct sdw_bus *bus = sdw->bus;
struct hid_device *hid;
- struct sdw_slave *slave = dev_to_sdw_dev(dev);
int ret;
- bus = slave->bus;
-
hid = hid_allocate_device();
if (IS_ERR(hid))
return PTR_ERR(hid);
@@ -103,8 +101,8 @@ int sdca_add_hid_device(struct device *dev, struct sdca_entity *entity)
snprintf(hid->name, sizeof(hid->name),
"HID sdw:%01x:%01x:%04x:%04x:%02x",
- bus->controller_id, bus->link_id, slave->id.mfg_id,
- slave->id.part_id, slave->id.class_id);
+ bus->controller_id, bus->link_id, sdw->id.mfg_id,
+ sdw->id.part_id, sdw->id.class_id);
snprintf(hid->phys, sizeof(hid->phys), "%s", dev->bus->name);
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 05/19] ASoC: SDCA: Pass device register map from IRQ alloc to handlers
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (3 preceding siblings ...)
2025-10-20 15:54 ` [PATCH v3 RESEND 04/19] ASoC: SDCA: Pass SoundWire slave to HID Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:54 ` [PATCH v3 RESEND 06/19] ASoC: SDCA: Update externally_requested flag to cover all requests Charles Keepax
` (15 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Store a copy of the device register map in the structure for the IRQ
handlers. This will allow the individual IRQ handlers access to the
device level register map if required.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_interrupts.h | 2 ++
sound/soc/sdca/sdca_interrupts.c | 5 ++++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/sound/sdca_interrupts.h b/include/sound/sdca_interrupts.h
index bbbc3ab27ebae..d652c6e94ddcb 100644
--- a/include/sound/sdca_interrupts.h
+++ b/include/sound/sdca_interrupts.h
@@ -23,6 +23,7 @@ struct sdca_function_data;
/**
* struct sdca_interrupt - contains information about a single SDCA interrupt
* @name: The name of the interrupt.
+ * @device_regmap: Pointer to the IRQ regmap.
* @component: Pointer to the ASoC component owns the interrupt.
* @function: Pointer to the Function that the interrupt is associated with.
* @entity: Pointer to the Entity that the interrupt is associated with.
@@ -35,6 +36,7 @@ struct sdca_function_data;
struct sdca_interrupt {
const char *name;
+ struct regmap *device_regmap;
struct snd_soc_component *component;
struct sdca_function_data *function;
struct sdca_entity *entity;
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 9295d283be910..898069ceffe93 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -437,7 +437,7 @@ struct sdca_interrupt_info *sdca_irq_allocate(struct device *sdev,
struct regmap *regmap, int irq)
{
struct sdca_interrupt_info *info;
- int ret;
+ int ret, i;
info = devm_kzalloc(sdev, sizeof(*info), GFP_KERNEL);
if (!info)
@@ -445,6 +445,9 @@ struct sdca_interrupt_info *sdca_irq_allocate(struct device *sdev,
info->irq_chip = sdca_irq_chip;
+ for (i = 0; i < ARRAY_SIZE(info->irqs); i++)
+ info->irqs[i].device_regmap = regmap;
+
ret = devm_mutex_init(sdev, &info->irq_lock);
if (ret)
return ERR_PTR(ret);
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 06/19] ASoC: SDCA: Update externally_requested flag to cover all requests
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (4 preceding siblings ...)
2025-10-20 15:54 ` [PATCH v3 RESEND 05/19] ASoC: SDCA: Pass device register map from IRQ alloc to handlers Charles Keepax
@ 2025-10-20 15:54 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 07/19] ASoC: SDCA: Factor out a helper to find SDCA IRQ data Charles Keepax
` (14 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:54 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Currently there is a flag to indicate if an IRQ has been requested by
something outside the SDCA core, such that the core can skip requesting
that IRQ. However, it is simpler and more useful to always store the
allocated IRQ number. This will allow the core to see if the IRQ has
been requested, to perform additional operations on the IRQ, and
request IRQs in multiple phases.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Changes since v2:
- Use IRQ number rather than a boolean for tracking used IRQs.
include/sound/sdca_interrupts.h | 7 +++----
sound/soc/sdca/sdca_interrupts.c | 8 ++++----
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/sound/sdca_interrupts.h b/include/sound/sdca_interrupts.h
index d652c6e94ddcb..e4bf123936bbd 100644
--- a/include/sound/sdca_interrupts.h
+++ b/include/sound/sdca_interrupts.h
@@ -29,9 +29,8 @@ struct sdca_function_data;
* @entity: Pointer to the Entity that the interrupt is associated with.
* @control: Pointer to the Control that the interrupt is associated with.
* @priv: Pointer to private data for use by the handler.
- * @externally_requested: Internal flag used to check if a client driver has
- * already requested the interrupt, for custom handling, allowing the core to
- * skip handling this interrupt.
+ * @irq: IRQ number allocated to this interrupt, also used internally to track
+ * the IRQ being assigned.
*/
struct sdca_interrupt {
const char *name;
@@ -44,7 +43,7 @@ struct sdca_interrupt {
void *priv;
- bool externally_requested;
+ int irq;
};
/**
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 898069ceffe93..cb7c7a6f356ed 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -262,6 +262,8 @@ static int sdca_irq_request_locked(struct device *dev,
if (ret)
return ret;
+ info->irqs[sdca_irq].irq = irq;
+
dev_dbg(dev, "requested irq %d for %s\n", irq, name);
return 0;
@@ -301,8 +303,6 @@ int sdca_irq_request(struct device *dev, struct sdca_interrupt_info *info,
return ret;
}
- info->irqs[sdca_irq].externally_requested = true;
-
return 0;
}
EXPORT_SYMBOL_NS_GPL(sdca_irq_request, "SND_SOC_SDCA");
@@ -379,9 +379,9 @@ int sdca_irq_populate(struct sdca_function_data *function,
interrupt = &info->irqs[irq];
- if (interrupt->externally_requested) {
+ if (interrupt->requested) {
dev_dbg(dev,
- "skipping irq %d, externally requested\n",
+ "skipping irq %d, already requested\n",
irq);
continue;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 07/19] ASoC: SDCA: Factor out a helper to find SDCA IRQ data
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (5 preceding siblings ...)
2025-10-20 15:54 ` [PATCH v3 RESEND 06/19] ASoC: SDCA: Update externally_requested flag to cover all requests Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 08/19] ASoC: SDCA: Rely less on the ASoC component in IRQ handling Charles Keepax
` (13 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Add a helper function to locate the sdca_interrupt structure for a given
SDCA IRQ, this makes the code a little more readable and will facilitate
some additions in the future.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
sound/soc/sdca/sdca_interrupts.c | 36 +++++++++++++++++++-------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index cb7c7a6f356ed..2b3bb7d0cb443 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -341,6 +341,24 @@ int sdca_irq_data_populate(struct snd_soc_component *component,
}
EXPORT_SYMBOL_NS_GPL(sdca_irq_data_populate, "SND_SOC_SDCA");
+static struct sdca_interrupt *get_interrupt_data(struct device *dev, int irq,
+ struct sdca_interrupt_info *info)
+{
+ if (irq == SDCA_NO_INTERRUPT) {
+ return NULL;
+ } else if (irq < 0 || irq >= SDCA_MAX_INTERRUPTS) {
+ dev_err(dev, "bad irq position: %d\n", irq);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (info->irqs[irq].irq) {
+ dev_dbg(dev, "skipping irq %d, already requested\n", irq);
+ return NULL;
+ }
+
+ return &info->irqs[irq];
+}
+
/**
* sdca_irq_populate - Request all the individual IRQs for an SDCA Function
* @function: Pointer to the SDCA Function.
@@ -370,21 +388,11 @@ int sdca_irq_populate(struct sdca_function_data *function,
irq_handler_t handler;
int ret;
- if (irq == SDCA_NO_INTERRUPT) {
- continue;
- } else if (irq < 0 || irq >= SDCA_MAX_INTERRUPTS) {
- dev_err(dev, "bad irq position: %d\n", irq);
- return -EINVAL;
- }
-
- interrupt = &info->irqs[irq];
-
- if (interrupt->requested) {
- dev_dbg(dev,
- "skipping irq %d, already requested\n",
- irq);
+ interrupt = get_interrupt_data(dev, irq, info);
+ if (IS_ERR(interrupt))
+ return PTR_ERR(interrupt);
+ else if (!interrupt)
continue;
- }
ret = sdca_irq_data_populate(component, function, entity,
control, interrupt);
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 08/19] ASoC: SDCA: Rely less on the ASoC component in IRQ handling
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (6 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 07/19] ASoC: SDCA: Factor out a helper to find SDCA IRQ data Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 09/19] ASoC: SDCA: Force some SDCA Controls to be volatile Charles Keepax
` (12 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
In the future some IRQs (mostly the UMPs used during File
DownLoad) will need to run after the device has enumerated on the
bus but before the soundcard is actually constructed. As such
refactor more of the IRQ handling to use raw device and regmap
pointers, rather than accessing things through the component.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_interrupts.h | 7 +++++-
sound/soc/sdca/sdca_interrupts.c | 37 +++++++++++++++++++++-----------
2 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/include/sound/sdca_interrupts.h b/include/sound/sdca_interrupts.h
index e4bf123936bbd..3983f515349ad 100644
--- a/include/sound/sdca_interrupts.h
+++ b/include/sound/sdca_interrupts.h
@@ -23,7 +23,9 @@ struct sdca_function_data;
/**
* struct sdca_interrupt - contains information about a single SDCA interrupt
* @name: The name of the interrupt.
+ * @dev: Pointer to the Function device.
* @device_regmap: Pointer to the IRQ regmap.
+ * @function_regmap: Pointer to the SDCA Function regmap.
* @component: Pointer to the ASoC component owns the interrupt.
* @function: Pointer to the Function that the interrupt is associated with.
* @entity: Pointer to the Entity that the interrupt is associated with.
@@ -35,7 +37,9 @@ struct sdca_function_data;
struct sdca_interrupt {
const char *name;
+ struct device *dev;
struct regmap *device_regmap;
+ struct regmap *function_regmap;
struct snd_soc_component *component;
struct sdca_function_data *function;
struct sdca_entity *entity;
@@ -65,7 +69,8 @@ struct sdca_interrupt_info {
int sdca_irq_request(struct device *dev, struct sdca_interrupt_info *interrupt_info,
int sdca_irq, const char *name, irq_handler_t handler,
void *data);
-int sdca_irq_data_populate(struct snd_soc_component *component,
+int sdca_irq_data_populate(struct device *dev, struct regmap *function_regmap,
+ struct snd_soc_component *component,
struct sdca_function_data *function,
struct sdca_entity *entity,
struct sdca_control *control,
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 2b3bb7d0cb443..d0894c8e0552c 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -77,7 +77,7 @@ static const struct regmap_irq_chip sdca_irq_chip = {
static irqreturn_t base_handler(int irq, void *data)
{
struct sdca_interrupt *interrupt = data;
- struct device *dev = interrupt->component->dev;
+ struct device *dev = interrupt->dev;
dev_info(dev, "%s irq without full handling\n", interrupt->name);
@@ -87,7 +87,7 @@ static irqreturn_t base_handler(int irq, void *data)
static irqreturn_t function_status_handler(int irq, void *data)
{
struct sdca_interrupt *interrupt = data;
- struct device *dev = interrupt->component->dev;
+ struct device *dev = interrupt->dev;
irqreturn_t irqret = IRQ_NONE;
unsigned int reg, val;
unsigned long status;
@@ -103,7 +103,7 @@ static irqreturn_t function_status_handler(int irq, void *data)
reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
interrupt->control->sel, 0);
- ret = regmap_read(interrupt->component->regmap, reg, &val);
+ ret = regmap_read(interrupt->function_regmap, reg, &val);
if (ret < 0) {
dev_err(dev, "failed to read function status: %d\n", ret);
goto error;
@@ -136,7 +136,7 @@ static irqreturn_t function_status_handler(int irq, void *data)
}
}
- ret = regmap_write(interrupt->component->regmap, reg, val);
+ ret = regmap_write(interrupt->function_regmap, reg, val);
if (ret < 0) {
dev_err(dev, "failed to clear function status: %d\n", ret);
goto error;
@@ -151,8 +151,8 @@ static irqreturn_t function_status_handler(int irq, void *data)
static irqreturn_t detected_mode_handler(int irq, void *data)
{
struct sdca_interrupt *interrupt = data;
+ struct device *dev = interrupt->dev;
struct snd_soc_component *component = interrupt->component;
- struct device *dev = component->dev;
struct snd_soc_card *card = component->card;
struct rw_semaphore *rwsem = &card->snd_card->controls_rwsem;
struct snd_kcontrol *kctl = interrupt->priv;
@@ -190,7 +190,7 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
interrupt->control->sel, 0);
- ret = regmap_read(component->regmap, reg, &val);
+ ret = regmap_read(interrupt->function_regmap, reg, &val);
if (ret < 0) {
dev_err(dev, "failed to read detected mode: %d\n", ret);
goto error;
@@ -209,9 +209,9 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
* detected mode is unknown we need to see what the device
* selected as a "safe" option.
*/
- regcache_drop_region(component->regmap, reg, reg);
+ regcache_drop_region(interrupt->function_regmap, reg, reg);
- ret = regmap_read(component->regmap, reg, &val);
+ ret = regmap_read(interrupt->function_regmap, reg, &val);
if (ret) {
dev_err(dev, "failed to re-check selected mode: %d\n", ret);
goto error;
@@ -309,6 +309,8 @@ EXPORT_SYMBOL_NS_GPL(sdca_irq_request, "SND_SOC_SDCA");
/**
* sdca_irq_data_populate - Populate common interrupt data
+ * @dev: Pointer to the Function device.
+ * @regmap: Pointer to the Function regmap.
* @component: Pointer to the ASoC component for the Function.
* @function: Pointer to the SDCA Function.
* @entity: Pointer to the SDCA Entity.
@@ -317,21 +319,31 @@ EXPORT_SYMBOL_NS_GPL(sdca_irq_request, "SND_SOC_SDCA");
*
* Return: Zero on success, and a negative error code on failure.
*/
-int sdca_irq_data_populate(struct snd_soc_component *component,
+int sdca_irq_data_populate(struct device *dev, struct regmap *regmap,
+ struct snd_soc_component *component,
struct sdca_function_data *function,
struct sdca_entity *entity,
struct sdca_control *control,
struct sdca_interrupt *interrupt)
{
- struct device *dev = component->dev;
const char *name;
+ if (!dev && component)
+ dev = component->dev;
+ if (!dev)
+ return -ENODEV;
+
name = devm_kasprintf(dev, GFP_KERNEL, "%s %s %s", function->desc->name,
entity->label, control->label);
if (!name)
return -ENOMEM;
interrupt->name = name;
+ interrupt->dev = dev;
+ if (!regmap && component)
+ interrupt->function_regmap = component->regmap;
+ else
+ interrupt->function_regmap = regmap;
interrupt->component = component;
interrupt->function = function;
interrupt->entity = entity;
@@ -394,8 +406,9 @@ int sdca_irq_populate(struct sdca_function_data *function,
else if (!interrupt)
continue;
- ret = sdca_irq_data_populate(component, function, entity,
- control, interrupt);
+ ret = sdca_irq_data_populate(dev, NULL, component,
+ function, entity, control,
+ interrupt);
if (ret)
return ret;
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 09/19] ASoC: SDCA: Force some SDCA Controls to be volatile
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (7 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 08/19] ASoC: SDCA: Rely less on the ASoC component in IRQ handling Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 10/19] ASoC: SDCA: Parse XU Entity properties Charles Keepax
` (11 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Whilst SDCA does specify an Access Mode for each Control, there is not a
1-to-1 mapping between that and ASoC's internal representation. Some
registers require being treated as volatile from the hosts perspective
even in their Access Mode is Read-Write. Add an explicit list of SDCA
controls that should be forced volatile.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Changes since v2:
- Reorder controls by entity
include/sound/sdca_function.h | 1 +
sound/soc/sdca/sdca_functions.c | 58 +++++++++++++++++++++++++++++++++
sound/soc/sdca/sdca_regmap.c | 9 +----
3 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index 51e12fcfc53cd..ab9af84082c95 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -771,6 +771,7 @@ struct sdca_control {
u8 layers;
bool deferrable;
+ bool is_volatile;
bool has_default;
bool has_fixed;
};
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 2d5d20e23e3c4..3e1df30f5d609 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -779,6 +779,62 @@ find_sdca_control_datatype(const struct sdca_entity *entity,
}
}
+static bool find_sdca_control_volatile(const struct sdca_entity *entity,
+ const struct sdca_control *control)
+{
+ switch (control->mode) {
+ case SDCA_ACCESS_MODE_DC:
+ return false;
+ case SDCA_ACCESS_MODE_RO:
+ case SDCA_ACCESS_MODE_RW1S:
+ case SDCA_ACCESS_MODE_RW1C:
+ return true;
+ default:
+ break;
+ }
+
+ switch (SDCA_CTL_TYPE(entity->type, control->sel)) {
+ case SDCA_CTL_TYPE_S(XU, FDL_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(XU, FDL_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(XU, FDL_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(XU, FDL_STATUS):
+ case SDCA_CTL_TYPE_S(XU, FDL_HOST_REQUEST):
+ case SDCA_CTL_TYPE_S(SPE, AUTHTX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SPE, AUTHTX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SPE, AUTHTX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SPE, AUTHRX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SPE, AUTHRX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SPE, AUTHRX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(MFPU, AE_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(MFPU, AE_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(MFPU, AE_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SMPU, HIST_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SMPU, HIST_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SMPU, HIST_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SMPU, DTODTX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SMPU, DTODTX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SMPU, DTODTX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SMPU, DTODRX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SMPU, DTODRX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SMPU, DTODRX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SAPU, DTODTX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SAPU, DTODTX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SAPU, DTODTX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(SAPU, DTODRX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(SAPU, DTODRX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(SAPU, DTODRX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(HIDE, HIDTX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(HIDE, HIDTX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(HIDE, HIDTX_MESSAGELENGTH):
+ case SDCA_CTL_TYPE_S(HIDE, HIDRX_CURRENTOWNER):
+ case SDCA_CTL_TYPE_S(HIDE, HIDRX_MESSAGEOFFSET):
+ case SDCA_CTL_TYPE_S(HIDE, HIDRX_MESSAGELENGTH):
+ return true;
+ default:
+ return false;
+ }
+}
+
static int find_sdca_control_range(struct device *dev,
struct fwnode_handle *control_node,
struct sdca_control_range *range)
@@ -930,6 +986,8 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
break;
}
+ control->is_volatile = find_sdca_control_volatile(entity, control);
+
ret = find_sdca_control_range(dev, control_node, &control->range);
if (ret) {
dev_err(dev, "%s: control %#x: range missing: %d\n",
diff --git a/sound/soc/sdca/sdca_regmap.c b/sound/soc/sdca/sdca_regmap.c
index 72f893e00ff50..8fa138fca00ff 100644
--- a/sound/soc/sdca/sdca_regmap.c
+++ b/sound/soc/sdca/sdca_regmap.c
@@ -147,14 +147,7 @@ bool sdca_regmap_volatile(struct sdca_function_data *function, unsigned int reg)
if (!control)
return false;
- switch (control->mode) {
- case SDCA_ACCESS_MODE_RO:
- case SDCA_ACCESS_MODE_RW1S:
- case SDCA_ACCESS_MODE_RW1C:
- return true;
- default:
- return false;
- }
+ return control->is_volatile;
}
EXPORT_SYMBOL_NS(sdca_regmap_volatile, "SND_SOC_SDCA");
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 10/19] ASoC: SDCA: Parse XU Entity properties
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (8 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 09/19] ASoC: SDCA: Force some SDCA Controls to be volatile Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 11/19] ASoC: SDCA: Parse Function Reset max delay Charles Keepax
` (10 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Parse the DisCo properties for XU Entities.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_function.h | 23 +++++++++++++++++++++++
sound/soc/sdca/sdca_functions.c | 25 +++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index ab9af84082c95..f2ce13162151d 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -1090,6 +1090,27 @@ struct sdca_entity_hide {
struct hid_descriptor hid_desc;
};
+/**
+ * enum sdca_xu_reset_machanism - SDCA FDL Resets
+ */
+enum sdca_xu_reset_mechanism {
+ SDCA_XU_RESET_FUNCTION = 0x0,
+ SDCA_XU_RESET_DEVICE = 0x1,
+ SDCA_XU_RESET_BUS = 0x2,
+};
+
+/**
+ * struct sdca_entity_xu - information specific to XU Entities
+ * @max_delay: the maximum time in microseconds allowed for the Device
+ * to change the ownership from Device to Host
+ * @reset_mechanism: indicates the type of reset that can be requested
+ * the end of an FDL.
+ */
+struct sdca_entity_xu {
+ unsigned int max_delay;
+ enum sdca_xu_reset_mechanism reset_mechanism;
+};
+
/**
* struct sdca_entity - information for one SDCA Entity
* @label: String such as "OT 12".
@@ -1106,6 +1127,7 @@ struct sdca_entity_hide {
* @pde: Power Domain Entity specific Entity properties.
* @ge: Group Entity specific Entity properties.
* @hide: HIDE Entity specific Entity properties.
+ * @xu: XU Entity specific Entity properties.
*/
struct sdca_entity {
const char *label;
@@ -1123,6 +1145,7 @@ struct sdca_entity {
struct sdca_entity_pde pde;
struct sdca_entity_ge ge;
struct sdca_entity_hide hide;
+ struct sdca_entity_xu xu;
};
};
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 3e1df30f5d609..2e66748462213 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -1398,6 +1398,28 @@ find_sdca_entity_hide(struct device *dev, struct sdw_slave *sdw,
return 0;
}
+static int find_sdca_entity_xu(struct device *dev,
+ struct fwnode_handle *entity_node,
+ struct sdca_entity *entity)
+{
+ struct sdca_entity_xu *xu = &entity->xu;
+ u32 tmp;
+ int ret;
+
+ ret = fwnode_property_read_u32(entity_node,
+ "mipi-sdca-RxUMP-ownership-transition-max-delay",
+ &tmp);
+ if (!ret)
+ xu->max_delay = tmp;
+
+ ret = fwnode_property_read_u32(entity_node, "mipi-sdca-FDL-reset-mechanism",
+ &tmp);
+ if (!ret)
+ xu->reset_mechanism = tmp;
+
+ return 0;
+}
+
static int find_sdca_entity(struct device *dev, struct sdw_slave *sdw,
struct fwnode_handle *function_node,
struct fwnode_handle *entity_node,
@@ -1430,6 +1452,9 @@ static int find_sdca_entity(struct device *dev, struct sdw_slave *sdw,
case SDCA_ENTITY_TYPE_OT:
ret = find_sdca_entity_iot(dev, entity_node, entity);
break;
+ case SDCA_ENTITY_TYPE_XU:
+ ret = find_sdca_entity_xu(dev, entity_node, entity);
+ break;
case SDCA_ENTITY_TYPE_CS:
ret = find_sdca_entity_cs(dev, entity_node, entity);
break;
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 11/19] ASoC: SDCA: Parse Function Reset max delay
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (9 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 10/19] ASoC: SDCA: Parse XU Entity properties Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 12/19] ASoC: SDCA: Add UMP buffer helper functions Charles Keepax
` (9 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Parse the DisCo property to get the timeout for a Function Reset.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_function.h | 3 +++
sound/soc/sdca/sdca_functions.c | 10 ++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index f2ce13162151d..2e988a30481c7 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -1323,6 +1323,8 @@ enum sdca_cluster_range {
* @num_clusters: Number of Channel Clusters reported in this Function.
* @busy_max_delay: Maximum Function busy delay in microseconds, before an
* error should be reported.
+ * @reset_max_delay: Maximum Function reset delay in microseconds, before an
+ * error should be reported.
*/
struct sdca_function_data {
struct sdca_function_desc *desc;
@@ -1335,6 +1337,7 @@ struct sdca_function_data {
int num_clusters;
unsigned int busy_max_delay;
+ unsigned int reset_max_delay;
};
static inline u32 sdca_range(struct sdca_control_range *range,
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 2e66748462213..6602727c73f73 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -2033,8 +2033,14 @@ int sdca_parse_function(struct device *dev, struct sdw_slave *sdw,
if (!ret)
function->busy_max_delay = tmp;
- dev_info(dev, "%pfwP: name %s delay %dus\n", function->desc->node,
- function->desc->name, function->busy_max_delay);
+ ret = fwnode_property_read_u32(function_desc->node,
+ "mipi-sdca-function-reset-max-delay", &tmp);
+ if (!ret)
+ function->reset_max_delay = tmp;
+
+ dev_info(dev, "%pfwP: name %s busy delay %dus reset delay %dus\n",
+ function->desc->node, function->desc->name,
+ function->busy_max_delay, function->reset_max_delay);
ret = find_sdca_init_table(dev, function_desc->node, function);
if (ret)
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 12/19] ASoC: SDCA: Add UMP buffer helper functions
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (10 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 11/19] ASoC: SDCA: Parse Function Reset max delay Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 13/19] ASoC: SDCA: Add SDCA FDL data parsing Charles Keepax
` (8 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Add helper functions for handling Universal Message Passing (UMP)
buffers on SDCA devices. These are generic mechanisms to pass blocks of
binary data between the host and the device, in both directions. They
are used for things like passing HID descriptors and the File Download
process.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_function.h | 26 ++++
include/sound/sdca_ump.h | 45 +++++++
sound/soc/sdca/Makefile | 3 +-
sound/soc/sdca/sdca_ump.c | 247 ++++++++++++++++++++++++++++++++++
4 files changed, 320 insertions(+), 1 deletion(-)
create mode 100644 include/sound/sdca_ump.h
create mode 100644 sound/soc/sdca/sdca_ump.c
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index 2e988a30481c7..6dd44a7a8a359 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -133,6 +133,32 @@ struct sdca_init_write {
#define SDCA_CTL_TYPE_S(ent, sel) SDCA_CTL_TYPE(SDCA_ENTITY_TYPE_##ent, \
SDCA_CTL_##ent##_##sel)
+/**
+ * enum sdca_messageoffset_range - Column definitions UMP MessageOffset
+ */
+enum sdca_messageoffset_range {
+ SDCA_MESSAGEOFFSET_BUFFER_START_ADDRESS = 0,
+ SDCA_MESSAGEOFFSET_BUFFER_LENGTH = 1,
+ SDCA_MESSAGEOFFSET_UMP_MODE = 2,
+ SDCA_MESSAGEOFFSET_NCOLS = 3,
+};
+
+/**
+ * enum sdca_ump_mode - SDCA UMP Mode
+ */
+enum sdca_ump_mode {
+ SDCA_UMP_MODE_DIRECT = 0x00,
+ SDCA_UMP_MODE_INDIRECT = 0x01,
+};
+
+/**
+ * enum sdca_ump_owner - SDCA UMP Owner
+ */
+enum sdca_ump_owner {
+ SDCA_UMP_OWNER_HOST = 0x00,
+ SDCA_UMP_OWNER_DEVICE = 0x01,
+};
+
/**
* enum sdca_it_controls - SDCA Controls for Input Terminal
*
diff --git a/include/sound/sdca_ump.h b/include/sound/sdca_ump.h
new file mode 100644
index 0000000000000..b2363199d19aa
--- /dev/null
+++ b/include/sound/sdca_ump.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * The MIPI SDCA specification is available for public downloads at
+ * https://www.mipi.org/mipi-sdca-v1-0-download
+ *
+ * Copyright (C) 2025 Cirrus Logic, Inc. and
+ * Cirrus Logic International Semiconductor Ltd.
+ */
+
+#ifndef __SDCA_UMP_H__
+#define __SDCA_UMP_H__
+
+struct regmap;
+struct sdca_control;
+struct sdca_entity;
+struct sdca_function_data;
+struct snd_soc_component;
+
+int sdca_ump_get_owner_host(struct device *dev,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ struct sdca_control *control);
+int sdca_ump_set_owner_device(struct device *dev,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ struct sdca_control *control);
+int sdca_ump_read_message(struct device *dev,
+ struct regmap *device_regmap,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ unsigned int offset_sel, unsigned int length_sel,
+ void **msg);
+int sdca_ump_write_message(struct device *dev,
+ struct regmap *device_regmap,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ unsigned int offset_sel, unsigned int msg_offset,
+ unsigned int length_sel,
+ void *msg, int msg_len);
+
+#endif // __SDCA_UMP_H__
diff --git a/sound/soc/sdca/Makefile b/sound/soc/sdca/Makefile
index 5e51760cb6513..a1b24c95cd8c8 100644
--- a/sound/soc/sdca/Makefile
+++ b/sound/soc/sdca/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
-snd-soc-sdca-y := sdca_functions.o sdca_device.o sdca_regmap.o sdca_asoc.o
+snd-soc-sdca-y := sdca_functions.o sdca_device.o sdca_regmap.o sdca_asoc.o \
+ sdca_ump.o
snd-soc-sdca-$(CONFIG_SND_SOC_SDCA_HID) += sdca_hid.o
snd-soc-sdca-$(CONFIG_SND_SOC_SDCA_IRQ) += sdca_interrupts.o
diff --git a/sound/soc/sdca/sdca_ump.c b/sound/soc/sdca/sdca_ump.c
new file mode 100644
index 0000000000000..5dcad2f7ea05b
--- /dev/null
+++ b/sound/soc/sdca/sdca_ump.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Cirrus Logic, Inc. and
+// Cirrus Logic International Semiconductor Ltd.
+
+/*
+ * The MIPI SDCA specification is available for public downloads at
+ * https://www.mipi.org/mipi-sdca-v1-0-download
+ */
+
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <sound/sdca.h>
+#include <sound/sdca_function.h>
+#include <sound/sdca_ump.h>
+#include <sound/soc-component.h>
+#include <linux/soundwire/sdw_registers.h>
+
+/**
+ * sdca_ump_get_owner_host - check a UMP buffer is owned by the host
+ * @dev: Pointer to the struct device used for error messages.
+ * @function_regmap: Pointer to the regmap for the SDCA Function.
+ * @function: Pointer to the Function information.
+ * @entity: Pointer to the SDCA Entity.
+ * @control: Pointer to the SDCA Control for the UMP Owner.
+ *
+ * Return: Returns zero on success, and a negative error code on failure.
+ */
+int sdca_ump_get_owner_host(struct device *dev,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ struct sdca_control *control)
+{
+ unsigned int reg, owner;
+ int ret;
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, control->sel, 0);
+ ret = regmap_read(function_regmap, reg, &owner);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to read UMP owner: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ if (owner != SDCA_UMP_OWNER_HOST) {
+ dev_err(dev, "%s: host is not the UMP owner\n", entity->label);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_get_owner_host, "SND_SOC_SDCA");
+
+/**
+ * sdca_ump_set_owner_device - set a UMP buffer's ownership back to the device
+ * @dev: Pointer to the struct device used for error messages.
+ * @function_regmap: Pointer to the regmap for the SDCA Function.
+ * @function: Pointer to the Function information.
+ * @entity: Pointer to the SDCA Entity.
+ * @control: Pointer to the SDCA Control for the UMP Owner.
+ *
+ * Return: Returns zero on success, and a negative error code on failure.
+ */
+int sdca_ump_set_owner_device(struct device *dev,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ struct sdca_control *control)
+{
+ unsigned int reg;
+ int ret;
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, control->sel, 0);
+ ret = regmap_write(function_regmap, reg, SDCA_UMP_OWNER_DEVICE);
+ if (ret < 0)
+ dev_err(dev, "%s: failed to write UMP owner: %d\n",
+ entity->label, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_set_owner_device, "SND_SOC_SDCA");
+
+/**
+ * sdca_ump_read_message - read a UMP message from the device
+ * @dev: Pointer to the struct device used for error messages.
+ * @device_regmap: Pointer to the Device register map.
+ * @function_regmap: Pointer to the regmap for the SDCA Function.
+ * @function: Pointer to the Function information.
+ * @entity: Pointer to the SDCA Entity.
+ * @offset_sel: Control Selector for the UMP Offset Control.
+ * @length_sel: Control Selector for the UMP Length Control.
+ * @msg: Pointer that will be populated with an dynamically buffer
+ * containing the UMP message. Note this needs to be freed by the
+ * caller.
+ *
+ * The caller should first call sdca_ump_get_owner_host() to ensure the host
+ * currently owns the UMP buffer, and then this function can be used to
+ * retrieve a message. It is the callers responsibility to free the
+ * message once it is finished with it. Finally sdca_ump_set_owner_device()
+ * should be called to return the buffer to the device.
+ *
+ * Return: Returns the message length on success, and a negative error
+ * code on failure.
+ */
+int sdca_ump_read_message(struct device *dev,
+ struct regmap *device_regmap,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ unsigned int offset_sel, unsigned int length_sel,
+ void **msg)
+{
+ struct sdca_control_range *range;
+ unsigned int msg_offset, msg_len;
+ unsigned int buf_addr, buf_len;
+ unsigned int reg;
+ int ret;
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, offset_sel, 0);
+ ret = regmap_read(function_regmap, reg, &msg_offset);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to read UMP offset: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ range = sdca_selector_find_range(dev, entity, offset_sel,
+ SDCA_MESSAGEOFFSET_NCOLS, 1);
+ if (!range)
+ return -ENOENT;
+
+ buf_addr = sdca_range(range, SDCA_MESSAGEOFFSET_BUFFER_START_ADDRESS, 0);
+ buf_len = sdca_range(range, SDCA_MESSAGEOFFSET_BUFFER_LENGTH, 0);
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, length_sel, 0);
+ ret = regmap_read(function_regmap, reg, &msg_len);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to read UMP length: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ if (msg_len > buf_len - msg_offset) {
+ dev_err(dev, "%s: message too big for UMP buffer: %d\n",
+ entity->label, msg_len);
+ return -EINVAL;
+ }
+
+ *msg = kmalloc(msg_len, GFP_KERNEL);
+ if (!*msg)
+ return -ENOMEM;
+
+ ret = regmap_raw_read(device_regmap, buf_addr + msg_offset, *msg, msg_len);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to read UMP message: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ return msg_len;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_read_message, "SND_SOC_SDCA");
+
+/**
+ * sdca_ump_write_message - write a UMP message to the device
+ * @dev: Pointer to the struct device used for error messages.
+ * @device_regmap: Pointer to the Device register map.
+ * @function_regmap: Pointer to the regmap for the SDCA Function.
+ * @function: Pointer to the Function information.
+ * @entity: Pointer to the SDCA Entity.
+ * @offset_sel: Control Selector for the UMP Offset Control.
+ * @msg_offset: Offset within the UMP buffer at which the message should
+ * be written.
+ * @length_sel: Control Selector for the UMP Length Control.
+ * @msg: Pointer to the data that should be written to the UMP buffer.
+ * @msg_len: Length of the message data in bytes.
+ *
+ * The caller should first call sdca_ump_get_owner_host() to ensure the host
+ * currently owns the UMP buffer, and then this function can be used to
+ * write a message. Finally sdca_ump_set_owner_device() should be called to
+ * return the buffer to the device, allowing the device to access the
+ * message.
+ *
+ * Return: Returns zero on success, and a negative error code on failure.
+ */
+int sdca_ump_write_message(struct device *dev,
+ struct regmap *device_regmap,
+ struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_entity *entity,
+ unsigned int offset_sel, unsigned int msg_offset,
+ unsigned int length_sel,
+ void *msg, int msg_len)
+{
+ struct sdca_control_range *range;
+ unsigned int buf_addr, buf_len, ump_mode;
+ unsigned int reg;
+ int ret;
+
+ range = sdca_selector_find_range(dev, entity, offset_sel,
+ SDCA_MESSAGEOFFSET_NCOLS, 1);
+ if (!range)
+ return -ENOENT;
+
+ buf_addr = sdca_range(range, SDCA_MESSAGEOFFSET_BUFFER_START_ADDRESS, 0);
+ buf_len = sdca_range(range, SDCA_MESSAGEOFFSET_BUFFER_LENGTH, 0);
+ ump_mode = sdca_range(range, SDCA_MESSAGEOFFSET_UMP_MODE, 0);
+
+ if (msg_len > buf_len - msg_offset) {
+ dev_err(dev, "%s: message too big for UMP buffer: %d\n",
+ entity->label, msg_len);
+ return -EINVAL;
+ }
+
+ if (ump_mode != SDCA_UMP_MODE_DIRECT) {
+ dev_err(dev, "%s: only direct mode currently supported\n",
+ entity->label);
+ return -EINVAL;
+ }
+
+ ret = regmap_raw_write(device_regmap, buf_addr + msg_offset, msg, msg_len);
+ if (ret) {
+ dev_err(dev, "%s: failed to write UMP message: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, offset_sel, 0);
+ ret = regmap_write(function_regmap, reg, msg_offset);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to write UMP offset: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ reg = SDW_SDCA_CTL(function->desc->adr, entity->id, length_sel, 0);
+ ret = regmap_write(function_regmap, reg, msg_len);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to write UMP length: %d\n",
+ entity->label, ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_write_message, "SND_SOC_SDCA");
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 13/19] ASoC: SDCA: Add SDCA FDL data parsing
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (11 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 12/19] ASoC: SDCA: Add UMP buffer helper functions Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities Charles Keepax
` (7 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
From: Maciej Strozek <mstrozek@opensource.cirrus.com>
Add parsing of ACPI DisCo information specific to FDL (File DownLoad).
DisCo contains a list of File Sets which can be requested by the device
and within each of those a list of individual files to be downloaded to
the device. Optionally the contents of the files may also be present in
a special ACPI table, called SWFT (SoundWire File Table).
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca.h | 5 ++
include/sound/sdca_function.h | 40 ++++++++++++++
sound/soc/sdca/sdca_device.c | 20 +++++++
sound/soc/sdca/sdca_functions.c | 93 +++++++++++++++++++++++++++++++++
4 files changed, 158 insertions(+)
diff --git a/include/sound/sdca.h b/include/sound/sdca.h
index 9c6a351c9d474..d38cdbfeb35f5 100644
--- a/include/sound/sdca.h
+++ b/include/sound/sdca.h
@@ -12,6 +12,7 @@
#include <linux/types.h>
#include <linux/kconfig.h>
+struct acpi_table_swft;
struct sdw_slave;
#define SDCA_MAX_FUNCTION_COUNT 8
@@ -37,11 +38,13 @@ struct sdca_function_desc {
* @num_functions: Total number of supported SDCA functions. Invalid/unsupported
* functions will be skipped.
* @function: Array of function descriptors.
+ * @swft: Pointer to the SWFT table, if available.
*/
struct sdca_device_data {
u32 interface_revision;
int num_functions;
struct sdca_function_desc function[SDCA_MAX_FUNCTION_COUNT];
+ struct acpi_table_swft *swft;
};
enum sdca_quirk {
@@ -52,12 +55,14 @@ enum sdca_quirk {
#if IS_ENABLED(CONFIG_ACPI) && 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);
#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)
{
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index 6dd44a7a8a359..f557206cec83d 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/hid.h>
+struct acpi_table_swft;
struct device;
struct sdca_entity;
struct sdca_function_desc;
@@ -1338,6 +1339,42 @@ enum sdca_cluster_range {
SDCA_CLUSTER_NCOLS = 2,
};
+/**
+ * struct sdca_fdl_file - information about a file from a fileset used in FDL
+ * @vendor_id: Vendor ID of the file.
+ * @file_id: File ID of the file.
+ * @fdl_offset: Offset information for FDL.
+ */
+struct sdca_fdl_file {
+ u16 vendor_id;
+ u32 file_id;
+ u32 fdl_offset;
+};
+
+/**
+ * struct sdca_fdl_set - information about a set of files used in FDL
+ * @files: Array of files in this FDL set.
+ * @num_files: Number of files in this FDL set.
+ * @id: ID of the FDL set.
+ */
+struct sdca_fdl_set {
+ struct sdca_fdl_file *files;
+ int num_files;
+ u32 id;
+};
+
+/**
+ * struct sdca_fdl_data - information about a function's FDL data
+ * @swft: Pointer to the SoundWire File Table.
+ * @sets: Array of FDL sets used by this function.
+ * @num_sets: Number of FDL sets used by this function.
+ */
+struct sdca_fdl_data {
+ struct acpi_table_swft *swft;
+ struct sdca_fdl_set *sets;
+ int num_sets;
+};
+
/**
* struct sdca_function_data - top-level information for one SDCA function
* @desc: Pointer to short descriptor from initial parsing.
@@ -1351,6 +1388,7 @@ enum sdca_cluster_range {
* error should be reported.
* @reset_max_delay: Maximum Function reset delay in microseconds, before an
* error should be reported.
+ * @fdl_data: FDL data for this Function, if available.
*/
struct sdca_function_data {
struct sdca_function_desc *desc;
@@ -1364,6 +1402,8 @@ struct sdca_function_data {
unsigned int busy_max_delay;
unsigned int reset_max_delay;
+
+ struct sdca_fdl_data fdl_data;
};
static inline u32 sdca_range(struct sdca_control_range *range,
diff --git a/sound/soc/sdca/sdca_device.c b/sound/soc/sdca/sdca_device.c
index 4798ce2c8f0b4..405e80b979de8 100644
--- a/sound/soc/sdca/sdca_device.c
+++ b/sound/soc/sdca/sdca_device.c
@@ -7,6 +7,7 @@
*/
#include <linux/acpi.h>
+#include <linux/device.h>
#include <linux/dmi.h>
#include <linux/module.h>
#include <linux/property.h>
@@ -27,6 +28,25 @@ void sdca_lookup_interface_revision(struct sdw_slave *slave)
}
EXPORT_SYMBOL_NS(sdca_lookup_interface_revision, "SND_SOC_SDCA");
+static void devm_acpi_table_put(void *ptr)
+{
+ acpi_put_table((struct acpi_table_header *)ptr);
+}
+
+void sdca_lookup_swft(struct sdw_slave *slave)
+{
+ acpi_status status;
+
+ status = acpi_get_table(ACPI_SIG_SWFT, 0,
+ (struct acpi_table_header **)&slave->sdca_data.swft);
+ if (ACPI_FAILURE(status))
+ dev_info(&slave->dev, "SWFT not available\n");
+ else
+ devm_add_action_or_reset(&slave->dev, devm_acpi_table_put,
+ &slave->sdca_data.swft);
+}
+EXPORT_SYMBOL_NS(sdca_lookup_swft, "SND_SOC_SDCA");
+
static bool sdca_device_quirk_rt712_vb(struct sdw_slave *slave)
{
struct sdw_slave_id *id = &slave->id;
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 6602727c73f73..b2e3fab9bd959 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -2010,6 +2010,95 @@ static int find_sdca_clusters(struct device *dev,
return 0;
}
+static int find_sdca_filesets(struct device *dev, struct sdw_slave *sdw,
+ struct fwnode_handle *function_node,
+ struct sdca_function_data *function)
+{
+ static const int mult_fileset = 3;
+ char fileset_name[SDCA_PROPERTY_LENGTH];
+ u32 *filesets_list __free(kfree) = NULL;
+ struct sdca_fdl_set *sets;
+ int num_sets;
+ int i, j;
+
+ num_sets = fwnode_property_count_u32(function_node,
+ "mipi-sdca-file-set-id-list");
+ if (num_sets == 0 || num_sets == -EINVAL) {
+ return 0;
+ } else if (num_sets < 0) {
+ dev_err(dev, "%pfwP: failed to read file set list: %d\n",
+ function_node, num_sets);
+ return num_sets;
+ }
+
+ filesets_list = kcalloc(num_sets, sizeof(u32), GFP_KERNEL);
+ if (!filesets_list)
+ return -ENOMEM;
+
+ fwnode_property_read_u32_array(function_node, "mipi-sdca-file-set-id-list",
+ filesets_list, num_sets);
+
+ sets = devm_kcalloc(dev, num_sets, sizeof(struct sdca_fdl_set), GFP_KERNEL);
+ if (!sets)
+ return -ENOMEM;
+
+ for (i = 0; i < num_sets; i++) {
+ u32 *fileset_entries __free(kfree) = NULL;
+ struct sdca_fdl_set *set = &sets[i];
+ struct sdca_fdl_file *files;
+ int num_files, num_entries;
+
+ snprintf(fileset_name, sizeof(fileset_name),
+ "mipi-sdca-file-set-id-0x%X", filesets_list[i]);
+
+ num_entries = fwnode_property_count_u32(function_node, fileset_name);
+ if (num_entries <= 0) {
+ dev_err(dev, "%pfwP: file set %d missing entries: %d\n",
+ function_node, filesets_list[i], num_entries);
+ return -EINVAL;
+ } else if (num_entries % mult_fileset != 0) {
+ dev_err(dev, "%pfwP: file set %d files not multiple of %d\n",
+ function_node, filesets_list[i], mult_fileset);
+ return -EINVAL;
+ }
+
+ dev_info(dev, "fileset: %#x\n", filesets_list[i]);
+
+ files = devm_kcalloc(dev, num_entries / mult_fileset,
+ sizeof(struct sdca_fdl_file), GFP_KERNEL);
+ if (!files)
+ return -ENOMEM;
+
+ fileset_entries = kcalloc(num_entries, sizeof(u32), GFP_KERNEL);
+ if (!fileset_entries)
+ return -ENOMEM;
+
+ fwnode_property_read_u32_array(function_node, fileset_name,
+ fileset_entries, num_entries);
+
+ for (j = 0, num_files = 0; j < num_entries; num_files++) {
+ struct sdca_fdl_file *file = &files[num_files];
+
+ file->vendor_id = fileset_entries[j++];
+ file->file_id = fileset_entries[j++];
+ file->fdl_offset = fileset_entries[j++];
+
+ dev_info(dev, "file: %#x, vendor: %#x, offset: %#x\n",
+ file->file_id, file->vendor_id, file->fdl_offset);
+ }
+
+ set->id = filesets_list[i];
+ set->num_files = num_files;
+ set->files = files;
+ }
+
+ function->fdl_data.swft = sdw->sdca_data.swft;
+ function->fdl_data.num_sets = num_sets;
+ function->fdl_data.sets = sets;
+
+ return 0;
+}
+
/**
* sdca_parse_function - parse ACPI DisCo for a Function
* @dev: Pointer to device against which function data will be allocated.
@@ -2058,6 +2147,10 @@ int sdca_parse_function(struct device *dev, struct sdw_slave *sdw,
if (ret < 0)
return ret;
+ ret = find_sdca_filesets(dev, sdw, function_desc->node, function);
+ if (ret)
+ return ret;
+
return 0;
}
EXPORT_SYMBOL_NS(sdca_parse_function, "SND_SOC_SDCA");
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (12 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 13/19] ASoC: SDCA: Add SDCA FDL data parsing Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-27 14:39 ` Pierre-Louis Bossart
2025-10-20 15:55 ` [PATCH v3 RESEND 15/19] ASoC: SDCA: Add FDL-specific IRQ processing Charles Keepax
` (6 subsequent siblings)
20 siblings, 1 reply; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
From: Maciej Strozek <mstrozek@opensource.cirrus.com>
Some instances of the XU Entity have a need for Files to be downloaded
from the Host. In these XUs, there is one instance of a Host to Device
(Consumer) UMP, identified by the FDL_CurrentOwner Control. FDL Library
introduced here implements the FDL flow triggered by FDL_CurrentOwner
irq, which sends a file from SoundWire File Table (SWFT) or from the
firmware directory in specific cases, to the Device FDL UMP.
Currently only Direct method of FDL is implemented.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Changes since v2:
- Check disk size of firmwares
- Reset on more FDL failure paths
include/sound/sdca_fdl.h | 58 ++++++
include/sound/sdca_function.h | 24 +++
sound/soc/sdca/Kconfig | 8 +
sound/soc/sdca/Makefile | 1 +
sound/soc/sdca/sdca_fdl.c | 376 ++++++++++++++++++++++++++++++++++
5 files changed, 467 insertions(+)
create mode 100644 include/sound/sdca_fdl.h
create mode 100644 sound/soc/sdca/sdca_fdl.c
diff --git a/include/sound/sdca_fdl.h b/include/sound/sdca_fdl.h
new file mode 100644
index 0000000000000..8b025aff4a0cd
--- /dev/null
+++ b/include/sound/sdca_fdl.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * The MIPI SDCA specification is available for public downloads at
+ * https://www.mipi.org/mipi-sdca-v1-0-download
+ *
+ * Copyright (C) 2025 Cirrus Logic, Inc. and
+ * Cirrus Logic International Semiconductor Ltd.
+ */
+
+#ifndef __SDCA_FDL_H__
+#define __SDCA_FDL_H__
+
+struct device;
+struct regmap;
+struct sdca_fdl_set;
+struct sdca_function_data;
+struct sdca_interrupt;
+
+/**
+ * struct fdl_state - FDL state structure to keep data between interrupts
+ * @set: Pointer to the FDL set currently being downloaded.
+ * @file_index: Index of the current file being processed.
+ */
+struct fdl_state {
+ struct sdca_fdl_set *set;
+ int file_index;
+};
+
+#define SDCA_CTL_XU_FDLH_COMPLETE 0
+#define SDCA_CTL_XU_FDLH_MORE_FILES SDCA_CTL_XU_FDLH_SET_IN_PROGRESS
+#define SDCA_CTL_XU_FDLH_FILE_AVAILABLE (SDCA_CTL_XU_FDLH_TRANSFERRED_FILE | \
+ SDCA_CTL_XU_FDLH_SET_IN_PROGRESS)
+#define SDCA_CTL_XU_FDLH_MASK (SDCA_CTL_XU_FDLH_TRANSFERRED_CHUNK | \
+ SDCA_CTL_XU_FDLH_TRANSFERRED_FILE | \
+ SDCA_CTL_XU_FDLH_SET_IN_PROGRESS | \
+ SDCA_CTL_XU_FDLH_RESET_ACK | \
+ SDCA_CTL_XU_FDLH_REQ_ABORT)
+
+#define SDCA_CTL_XU_FDLD_COMPLETE 0
+#define SDCA_CTL_XU_FDLD_FILE_OK (SDCA_CTL_XU_FDLH_TRANSFERRED_FILE | \
+ SDCA_CTL_XU_FDLH_SET_IN_PROGRESS | \
+ SDCA_CTL_XU_FDLD_ACK_TRANSFER | \
+ SDCA_CTL_XU_FDLD_NEEDS_SET)
+#define SDCA_CTL_XU_FDLD_MORE_FILES_OK (SDCA_CTL_XU_FDLH_SET_IN_PROGRESS | \
+ SDCA_CTL_XU_FDLD_ACK_TRANSFER | \
+ SDCA_CTL_XU_FDLD_NEEDS_SET)
+#define SDCA_CTL_XU_FDLD_MASK (SDCA_CTL_XU_FDLD_REQ_RESET | \
+ SDCA_CTL_XU_FDLD_REQ_ABORT | \
+ SDCA_CTL_XU_FDLD_ACK_TRANSFER | \
+ SDCA_CTL_XU_FDLD_NEEDS_SET)
+
+int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt);
+int sdca_fdl_process(struct sdca_interrupt *interrupt);
+
+int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
+ struct regmap *regmap);
+
+#endif // __SDCA_FDL_H__
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index f557206cec83d..99cb978f7099b 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -285,6 +285,27 @@ enum sdca_xu_controls {
SDCA_CTL_XU_FDL_STATUS = 0x14,
SDCA_CTL_XU_FDL_SET_INDEX = 0x15,
SDCA_CTL_XU_FDL_HOST_REQUEST = 0x16,
+
+ /* FDL Status Host->Device bit definitions */
+ SDCA_CTL_XU_FDLH_TRANSFERRED_CHUNK = BIT(0),
+ SDCA_CTL_XU_FDLH_TRANSFERRED_FILE = BIT(1),
+ SDCA_CTL_XU_FDLH_SET_IN_PROGRESS = BIT(2),
+ SDCA_CTL_XU_FDLH_RESET_ACK = BIT(4),
+ SDCA_CTL_XU_FDLH_REQ_ABORT = BIT(5),
+ /* FDL Status Device->Host bit definitions */
+ SDCA_CTL_XU_FDLD_REQ_RESET = BIT(4),
+ SDCA_CTL_XU_FDLD_REQ_ABORT = BIT(5),
+ SDCA_CTL_XU_FDLD_ACK_TRANSFER = BIT(6),
+ SDCA_CTL_XU_FDLD_NEEDS_SET = BIT(7),
+};
+
+/**
+ * enum sdca_set_index_range - Column definitions UMP SetIndex
+ */
+enum sdca_fdl_set_index_range {
+ SDCA_FDL_SET_INDEX_SET_NUMBER = 0,
+ SDCA_FDL_SET_INDEX_FILE_SET_ID = 1,
+ SDCA_FDL_SET_INDEX_NCOLS = 2,
};
/**
@@ -569,6 +590,9 @@ enum sdca_entity0_controls {
SDCA_CTL_ENTITY_0_FUNCTION_NEEDS_INITIALIZATION = BIT(5),
SDCA_CTL_ENTITY_0_FUNCTION_HAS_BEEN_RESET = BIT(6),
SDCA_CTL_ENTITY_0_FUNCTION_BUSY = BIT(7),
+
+ /* Function Action Bits */
+ SDCA_CTL_ENTITY_0_RESET_FUNCTION_NOW = BIT(0),
};
#define SDCA_CTL_MIC_BIAS_NAME "Mic Bias"
diff --git a/sound/soc/sdca/Kconfig b/sound/soc/sdca/Kconfig
index 6a3ba43f26bd9..a73920d07073e 100644
--- a/sound/soc/sdca/Kconfig
+++ b/sound/soc/sdca/Kconfig
@@ -25,6 +25,14 @@ config SND_SOC_SDCA_IRQ
help
This option enables support for SDCA IRQs.
+config SND_SOC_SDCA_FDL
+ bool "SDCA FDL (File DownLoad) support"
+ depends on SND_SOC_SDCA
+ default y
+ help
+ This option enables support for the File Download using UMP,
+ typically used for downloading firmware to devices.
+
config SND_SOC_SDCA_OPTIONAL
def_tristate SND_SOC_SDCA || !SND_SOC_SDCA
diff --git a/sound/soc/sdca/Makefile b/sound/soc/sdca/Makefile
index a1b24c95cd8c8..be911c399bbde 100644
--- a/sound/soc/sdca/Makefile
+++ b/sound/soc/sdca/Makefile
@@ -4,5 +4,6 @@ snd-soc-sdca-y := sdca_functions.o sdca_device.o sdca_regmap.o sdca_asoc.o \
sdca_ump.o
snd-soc-sdca-$(CONFIG_SND_SOC_SDCA_HID) += sdca_hid.o
snd-soc-sdca-$(CONFIG_SND_SOC_SDCA_IRQ) += sdca_interrupts.o
+snd-soc-sdca-$(CONFIG_SND_SOC_SDCA_FDL) += sdca_fdl.o
obj-$(CONFIG_SND_SOC_SDCA) += snd-soc-sdca.o
diff --git a/sound/soc/sdca/sdca_fdl.c b/sound/soc/sdca/sdca_fdl.c
new file mode 100644
index 0000000000000..8a15c6300556c
--- /dev/null
+++ b/sound/soc/sdca/sdca_fdl.c
@@ -0,0 +1,376 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Cirrus Logic, Inc. and
+// Cirrus Logic International Semiconductor Ltd.
+
+/*
+ * The MIPI SDCA specification is available for public downloads at
+ * https://www.mipi.org/mipi-sdca-v1-0-download
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/dmi.h>
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/regmap.h>
+#include <linux/sprintf.h>
+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_registers.h>
+#include <sound/sdca.h>
+#include <sound/sdca_fdl.h>
+#include <sound/sdca_function.h>
+#include <sound/sdca_interrupts.h>
+#include <sound/sdca_ump.h>
+
+/**
+ * sdca_reset_function - send an SDCA function reset
+ * @dev: Device pointer for error messages.
+ * @function: Pointer to the SDCA Function.
+ * @regmap: Pointer to the SDCA Function regmap.
+ *
+ * Return: Zero on success or a negative error code.
+ */
+int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
+ struct regmap *regmap)
+{
+ unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
+ SDCA_ENTITY_TYPE_ENTITY_0,
+ SDCA_CTL_ENTITY_0_FUNCTION_ACTION, 0);
+ unsigned int val, poll_us;
+ int ret;
+
+ ret = regmap_write(regmap, reg, SDCA_CTL_ENTITY_0_RESET_FUNCTION_NOW);
+ if (ret) // Allowed for function reset to not be implemented
+ return 0;
+
+ if (!function->reset_max_delay) {
+ dev_err(dev, "No reset delay specified in DisCo\n");
+ return -EINVAL;
+ }
+
+ poll_us = umin(function->reset_max_delay >> 4, 1000);
+
+ ret = regmap_read_poll_timeout(regmap, reg, val, !val, poll_us,
+ function->reset_max_delay);
+ if (ret) {
+ dev_err(dev, "Failed waiting for function reset: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS(sdca_reset_function, "SND_SOC_SDCA");
+
+static char *fdl_get_sku_filename(struct device *dev,
+ struct sdca_fdl_file *fdl_file)
+{
+ struct device *parent = dev;
+ const char *product_vendor;
+ const char *product_sku;
+
+ /*
+ * Try to find pci_dev manually because the card may not be ready to be
+ * used for snd_soc_card_get_pci_ssid yet
+ */
+ while (parent) {
+ if (dev_is_pci(parent)) {
+ struct pci_dev *pci_dev = to_pci_dev(parent);
+
+ return kasprintf(GFP_KERNEL, "sdca/%x/%x/%x/%x.bin",
+ fdl_file->vendor_id,
+ pci_dev->subsystem_vendor,
+ pci_dev->subsystem_device,
+ fdl_file->file_id);
+ } else {
+ parent = parent->parent;
+ }
+ }
+
+ product_vendor = dmi_get_system_info(DMI_SYS_VENDOR);
+ if (!product_vendor || !strcmp(product_vendor, "Default string"))
+ product_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
+ if (!product_vendor || !strcmp(product_vendor, "Default string"))
+ product_vendor = dmi_get_system_info(DMI_CHASSIS_VENDOR);
+ if (!product_vendor)
+ product_vendor = "unknown";
+
+ product_sku = dmi_get_system_info(DMI_PRODUCT_SKU);
+ if (!product_sku || !strcmp(product_sku, "Default string"))
+ product_sku = dmi_get_system_info(DMI_PRODUCT_NAME);
+ if (!product_sku)
+ product_sku = "unknown";
+
+ return kasprintf(GFP_KERNEL, "sdca/%x/%s/%s/%x.bin", fdl_file->vendor_id,
+ product_vendor, product_sku, fdl_file->file_id);
+}
+
+static int fdl_load_file(struct sdca_interrupt *interrupt,
+ struct sdca_fdl_set *set, int file_index)
+{
+ struct device *dev = interrupt->dev;
+ struct sdca_fdl_data *fdl_data = &interrupt->function->fdl_data;
+ const struct firmware *firmware = NULL;
+ struct acpi_sw_file *swf = NULL, *tmp;
+ struct sdca_fdl_file *fdl_file;
+ char *disk_filename;
+ int ret;
+ int i;
+
+ if (!set) {
+ dev_err(dev, "request to load SWF with no set\n");
+ return -EINVAL;
+ }
+
+ fdl_file = &set->files[file_index];
+
+ if (fdl_data->swft) {
+ tmp = fdl_data->swft->files;
+ for (i = 0; i < fdl_data->swft->header.length; i += tmp->file_length,
+ tmp = ACPI_ADD_PTR(struct acpi_sw_file, tmp, tmp->file_length)) {
+ if (tmp->vendor_id == fdl_file->vendor_id &&
+ tmp->file_id == fdl_file->file_id) {
+ dev_dbg(dev, "located SWF in ACPI: %x-%x-%x\n",
+ tmp->vendor_id, tmp->file_id,
+ tmp->file_version);
+ swf = tmp;
+ break;
+ }
+ }
+ }
+
+ disk_filename = fdl_get_sku_filename(dev, fdl_file);
+ if (!disk_filename)
+ return -ENOMEM;
+
+ dev_dbg(dev, "FDL disk filename: %s\n", disk_filename);
+
+ ret = firmware_request_nowarn(&firmware, disk_filename, dev);
+ kfree(disk_filename);
+ if (ret) {
+ disk_filename = kasprintf(GFP_KERNEL, "sdca/%x/%x.bin",
+ fdl_file->vendor_id, fdl_file->file_id);
+ if (!disk_filename)
+ return -ENOMEM;
+
+ dev_dbg(dev, "FDL disk filename: %s\n", disk_filename);
+
+ ret = firmware_request_nowarn(&firmware, disk_filename, dev);
+ kfree(disk_filename);
+ }
+
+ if (!ret) {
+ tmp = (struct acpi_sw_file *)&firmware->data[0];
+
+ if (firmware->size < sizeof(*tmp) ||
+ tmp->file_length != firmware->size) {
+ dev_err(dev, "bad disk SWF size\n");
+ } else if (!swf || swf->file_version <= tmp->file_version) {
+ dev_dbg(dev, "using SWF from disk: %x-%x-%x\n",
+ tmp->vendor_id, tmp->file_id, tmp->file_version);
+ swf = tmp;
+ }
+ }
+
+ if (!swf) {
+ dev_err(dev, "failed to locate SWF\n");
+ return -ENOENT;
+ }
+
+ ret = sdca_ump_write_message(dev, interrupt->device_regmap,
+ interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ SDCA_CTL_XU_FDL_MESSAGEOFFSET, fdl_file->fdl_offset,
+ SDCA_CTL_XU_FDL_MESSAGELENGTH, swf->data,
+ swf->file_length - offsetof(struct acpi_sw_file, data));
+ release_firmware(firmware);
+ return ret;
+}
+
+static struct sdca_fdl_set *fdl_get_set(struct sdca_interrupt *interrupt)
+{
+ struct device *dev = interrupt->dev;
+ struct sdca_fdl_data *fdl_data = &interrupt->function->fdl_data;
+ struct sdca_entity *xu = interrupt->entity;
+ struct sdca_control_range *range;
+ unsigned int val;
+ int i, ret;
+
+ ret = regmap_read(interrupt->function_regmap,
+ SDW_SDCA_CTL(interrupt->function->desc->adr, xu->id,
+ SDCA_CTL_XU_FDL_SET_INDEX, 0),
+ &val);
+ if (ret < 0) {
+ dev_err(dev, "failed to read FDL set index: %d\n", ret);
+ return NULL;
+ }
+
+ range = sdca_selector_find_range(dev, xu, SDCA_CTL_XU_FDL_SET_INDEX,
+ SDCA_FDL_SET_INDEX_NCOLS, 0);
+
+ val = sdca_range_search(range, SDCA_FDL_SET_INDEX_SET_NUMBER,
+ val, SDCA_FDL_SET_INDEX_FILE_SET_ID);
+
+ for (i = 0; i < fdl_data->num_sets; i++) {
+ if (fdl_data->sets[i].id == val)
+ return &fdl_data->sets[i];
+ }
+
+ dev_err(dev, "invalid fileset id: %d\n", val);
+ return NULL;
+}
+
+static void fdl_end(struct sdca_interrupt *interrupt)
+{
+ struct fdl_state *fdl_state = interrupt->priv;
+
+ if (!fdl_state->set)
+ return;
+
+ fdl_state->set = NULL;
+
+ dev_dbg(interrupt->dev, "completed FDL process\n");
+}
+
+static int fdl_status_process(struct sdca_interrupt *interrupt, unsigned int status)
+{
+ struct fdl_state *fdl_state = interrupt->priv;
+ int ret;
+
+ switch (status) {
+ case SDCA_CTL_XU_FDLD_NEEDS_SET:
+ dev_dbg(interrupt->dev, "starting FDL process...\n");
+
+ fdl_state->file_index = 0;
+ fdl_state->set = fdl_get_set(interrupt);
+ fallthrough;
+ case SDCA_CTL_XU_FDLD_MORE_FILES_OK:
+ ret = fdl_load_file(interrupt, fdl_state->set, fdl_state->file_index);
+ if (ret) {
+ fdl_end(interrupt);
+ return SDCA_CTL_XU_FDLH_REQ_ABORT;
+ }
+
+ return SDCA_CTL_XU_FDLH_FILE_AVAILABLE;
+ case SDCA_CTL_XU_FDLD_FILE_OK:
+ if (!fdl_state->set) {
+ fdl_end(interrupt);
+ return SDCA_CTL_XU_FDLH_REQ_ABORT;
+ }
+
+ fdl_state->file_index++;
+
+ if (fdl_state->file_index < fdl_state->set->num_files)
+ return SDCA_CTL_XU_FDLH_MORE_FILES;
+ fallthrough;
+ case SDCA_CTL_XU_FDLD_COMPLETE:
+ fdl_end(interrupt);
+ return SDCA_CTL_XU_FDLH_COMPLETE;
+ default:
+ fdl_end(interrupt);
+
+ if (status & SDCA_CTL_XU_FDLD_REQ_RESET)
+ return SDCA_CTL_XU_FDLH_RESET_ACK;
+ else if (status & SDCA_CTL_XU_FDLD_REQ_ABORT)
+ return SDCA_CTL_XU_FDLH_COMPLETE;
+
+ dev_err(interrupt->dev, "invalid FDL status: %x\n", status);
+ return -EINVAL;
+ }
+}
+
+/**
+ * sdca_fdl_process - Process the FDL state machine
+ * @interrupt: SDCA interrupt structure
+ *
+ * Based on section 13.2.5 Flow Diagram for File Download, Host side.
+ *
+ * Return: Zero on success or a negative error code.
+ */
+int sdca_fdl_process(struct sdca_interrupt *interrupt)
+{
+ struct device *dev = interrupt->dev;
+ struct sdca_entity_xu *xu = &interrupt->entity->xu;
+ unsigned int reg, status;
+ int response, ret;
+
+ ret = sdca_ump_get_owner_host(dev, interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ interrupt->control);
+ if (ret)
+ goto reset_function;
+
+ reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
+ SDCA_CTL_XU_FDL_STATUS, 0);
+ ret = regmap_read(interrupt->function_regmap, reg, &status);
+ if (ret < 0) {
+ dev_err(dev, "failed to read FDL status: %d\n", ret);
+ return ret;
+ }
+
+ dev_dbg(dev, "FDL status: %#x\n", status);
+
+ ret = fdl_status_process(interrupt, status);
+ if (ret < 0)
+ goto reset_function;
+
+ response = ret;
+
+ dev_dbg(dev, "FDL response: %#x\n", response);
+
+ ret = regmap_write(interrupt->function_regmap, reg,
+ response | (status & ~SDCA_CTL_XU_FDLH_MASK));
+ if (ret < 0) {
+ dev_err(dev, "failed to set FDL status signal: %d\n", ret);
+ return ret;
+ }
+
+ ret = sdca_ump_set_owner_device(dev, interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ interrupt->control);
+ if (ret)
+ return ret;
+
+ switch (response) {
+ case SDCA_CTL_XU_FDLH_RESET_ACK:
+ dev_dbg(dev, "FDL request reset\n");
+
+ switch (xu->reset_mechanism) {
+ default:
+ dev_warn(dev, "Requested reset mechanism not implemented\n");
+ fallthrough;
+ case SDCA_XU_RESET_FUNCTION:
+ goto reset_function;
+ }
+ default:
+ return 0;
+ }
+
+reset_function:
+ sdca_reset_function(dev, interrupt->function, interrupt->function_regmap);
+
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_fdl_process, "SND_SOC_SDCA");
+
+/**
+ * sdca_fdl_alloc_state - allocate state for an FDL interrupt
+ * @interrupt: SDCA interrupt structure.
+ *
+ * Return: Zero on success or a negative error code.
+ */
+int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt)
+{
+ struct device *dev = interrupt->dev;
+ struct fdl_state *fdl_state;
+
+ fdl_state = devm_kzalloc(dev, sizeof(struct fdl_state), GFP_KERNEL);
+ if (!fdl_state)
+ return -ENOMEM;
+
+ interrupt->priv = fdl_state;
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_fdl_alloc_state, "SND_SOC_SDCA");
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities
2025-10-20 15:55 ` [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities Charles Keepax
@ 2025-10-27 14:39 ` Pierre-Louis Bossart
2025-10-30 11:01 ` Charles Keepax
0 siblings, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2025-10-27 14:39 UTC (permalink / raw)
To: Charles Keepax, broonie
Cc: yung-chuan.liao, peter.ujfalusi, shumingf, lgirdwood, linux-sound,
patches
> +config SND_SOC_SDCA_FDL
> + bool "SDCA FDL (File DownLoad) support"
> + depends on SND_SOC_SDCA
> + default y
> + help
> + This option enables support for the File Download using UMP,
> + typically used for downloading firmware to devices.
nit-pick: shouldn't this option be selected by device drivers who need this library? There should be guardrails to prevent the fallback helpers from being used silently.
> +int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
> + struct regmap *regmap)
> +{
> + unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
> + SDCA_ENTITY_TYPE_ENTITY_0,
> + SDCA_CTL_ENTITY_0_FUNCTION_ACTION, 0);
> + unsigned int val, poll_us;
> + int ret;
> +
> + ret = regmap_write(regmap, reg, SDCA_CTL_ENTITY_0_RESET_FUNCTION_NOW);
> + if (ret) // Allowed for function reset to not be implemented
> + return 0;
nit-pick: maybe document why this is allowed. This doesn't seem very good in terms of design.
> +
> + if (!function->reset_max_delay) {
> + dev_err(dev, "No reset delay specified in DisCo\n");
> + return -EINVAL;
> + }
nit-pick: or maybe fallback to a reasonable default? Making the entire download dependent on Disco correctness isn't going to work long-term.
> +
> + poll_us = umin(function->reset_max_delay >> 4, 1000);
add comment on what this >> 4 means.
> +
> + ret = regmap_read_poll_timeout(regmap, reg, val, !val, poll_us,
> + function->reset_max_delay);
> + if (ret) {
> + dev_err(dev, "Failed waiting for function reset: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS(sdca_reset_function, "SND_SOC_SDCA");
> +/**
> + * sdca_fdl_process - Process the FDL state machine
> + * @interrupt: SDCA interrupt structure
> + *
> + * Based on section 13.2.5 Flow Diagram for File Download, Host side.
> + *
> + * Return: Zero on success or a negative error code.
> + */
> +int sdca_fdl_process(struct sdca_interrupt *interrupt)
> +{
> + struct device *dev = interrupt->dev;
> + struct sdca_entity_xu *xu = &interrupt->entity->xu;
> + unsigned int reg, status;
> + int response, ret;
> +
> + ret = sdca_ump_get_owner_host(dev, interrupt->function_regmap,
> + interrupt->function, interrupt->entity,
> + interrupt->control);
> + if (ret)
> + goto reset_function;
> +
> + reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
> + SDCA_CTL_XU_FDL_STATUS, 0);
> + ret = regmap_read(interrupt->function_regmap, reg, &status);
> + if (ret < 0) {
> + dev_err(dev, "failed to read FDL status: %d\n", ret);
> + return ret;
> + }
> +
> + dev_dbg(dev, "FDL status: %#x\n", status);
> +
> + ret = fdl_status_process(interrupt, status);
> + if (ret < 0)
> + goto reset_function;
> +
> + response = ret;
> +
> + dev_dbg(dev, "FDL response: %#x\n", response);
> +
> + ret = regmap_write(interrupt->function_regmap, reg,
> + response | (status & ~SDCA_CTL_XU_FDLH_MASK));
> + if (ret < 0) {
> + dev_err(dev, "failed to set FDL status signal: %d\n", ret);
> + return ret;
> + }
> +
> + ret = sdca_ump_set_owner_device(dev, interrupt->function_regmap,
> + interrupt->function, interrupt->entity,
> + interrupt->control);
> + if (ret)
> + return ret;
> +
> + switch (response) {
> + case SDCA_CTL_XU_FDLH_RESET_ACK:
> + dev_dbg(dev, "FDL request reset\n");
> +
> + switch (xu->reset_mechanism) {
> + default:
> + dev_warn(dev, "Requested reset mechanism not implemented\n");
> + fallthrough;
this fallthrough looks odd since it forces a check on the reset a second time ...
> + case SDCA_XU_RESET_FUNCTION:
> + goto reset_function;
> + }
> + default:
> + return 0;
> + }
> +
> +reset_function:
... inside this function call
> + sdca_reset_function(dev, interrupt->function, interrupt->function_regmap);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(sdca_fdl_process, "SND_SOC_SDCA");
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities
2025-10-27 14:39 ` Pierre-Louis Bossart
@ 2025-10-30 11:01 ` Charles Keepax
0 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-30 11:01 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: broonie, yung-chuan.liao, peter.ujfalusi, shumingf, lgirdwood,
linux-sound, patches
On Mon, Oct 27, 2025 at 03:39:40PM +0100, Pierre-Louis Bossart wrote:
> > +config SND_SOC_SDCA_FDL
> > + bool "SDCA FDL (File DownLoad) support"
> > + depends on SND_SOC_SDCA
> > + default y
> > + help
> > + This option enables support for the File Download using UMP,
> > + typically used for downloading firmware to devices.
>
> nit-pick: shouldn't this option be selected by device drivers
> who need this library?
These bits are all bools that add additional stuff into the main
SDCA module, so they do nothing if SDCA isn't selected. So the
device driver would just select SDCA generally speaking and I
think it makes sense if you enable SDCA that you get all the
functionality. If a certain config wants less they can disable
but that feels like its just for people really trying to size
optimise their kernel.
> There should be guardrails to prevent the fallback helpers from
> being used silently.
A top level driver has to bind to the device and it still
basically controls what happens, it has to register the stuff for
FDL to happen before it will happen.
> > +int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
> > + struct regmap *regmap)
> > +{
> > + unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
> > + SDCA_ENTITY_TYPE_ENTITY_0,
> > + SDCA_CTL_ENTITY_0_FUNCTION_ACTION, 0);
> > + unsigned int val, poll_us;
> > + int ret;
> > +
> > + ret = regmap_write(regmap, reg, SDCA_CTL_ENTITY_0_RESET_FUNCTION_NOW);
> > + if (ret) // Allowed for function reset to not be implemented
> > + return 0;
>
> nit-pick: maybe document why this is allowed. This doesn't seem
> very good in terms of design.
It's what the spec says, as often is the case I tend to agree with
you its not the best design decision. I guess perhaps the wording
could be tweaked but in general where the comments say this weird
thing is allowed/required they are referring to the spec.
> > + if (!function->reset_max_delay) {
> > + dev_err(dev, "No reset delay specified in DisCo\n");
> > + return -EINVAL;
> > + }
>
> nit-pick: or maybe fallback to a reasonable default? Making the
> entire download dependent on Disco correctness isn't going to work
> long-term.
I don't have any objections to a default here, although it should
also have a comment to say why, since the spec requires this
property if function reset is implemented.
On a wider point I am not sure I totally agree with the DisCo
correctness point. I guess we have to cross those bridges as we find
them but in general an awful lot of this class driver relies on DisCo
correctness. We build the whole function topology from DisCo if that
is incorrect there is a fairly good chance the part won't work.
Although for the most part my thinking has been if someone stuffs
up the DisCo too badly they will have to implement a custom
driver to handle it rather than relying on the class driver, but
for cases like this I think we can be a little flexible.
> > +
> > + poll_us = umin(function->reset_max_delay >> 4, 1000);
>
> add comment on what this >> 4 means.
Yeah that could use a comment I will add one.
> > + switch (response) {
> > + case SDCA_CTL_XU_FDLH_RESET_ACK:
> > + dev_dbg(dev, "FDL request reset\n");
> > +
> > + switch (xu->reset_mechanism) {
> > + default:
> > + dev_warn(dev, "Requested reset mechanism not implemented\n");
> > + fallthrough;
>
> this fallthrough looks odd since it forces a check on the reset a second time ...
>
> > + case SDCA_XU_RESET_FUNCTION:
> > + goto reset_function;
> > + }
> > + default:
> > + return 0;
> > + }
> > +
> > +reset_function:
>
> ... inside this function call
>
> > + sdca_reset_function(dev, interrupt->function, interrupt->function_regmap);
Not sure I totally follow this one, there isn't a second check of
reset_mechanism inside sdca_reset_function(). Perhaps this was an
earlier iteration of the code.
Thanks,
Charles
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 RESEND 15/19] ASoC: SDCA: Add FDL-specific IRQ processing
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (13 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 14/19] ASoC: SDCA: Add FDL library for XU entities Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 16/19] ASoC: SDCA: Add completion for FDL start and stop Charles Keepax
` (5 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
From: Maciej Strozek <mstrozek@opensource.cirrus.com>
Hookup the XU IRQs required for the FDL process.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
sound/soc/sdca/sdca_interrupts.c | 34 ++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index d0894c8e0552c..3a3b966b5782b 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -18,8 +18,10 @@
#include <linux/soundwire/sdw.h>
#include <linux/soundwire/sdw_registers.h>
#include <sound/sdca.h>
+#include <sound/sdca_fdl.h>
#include <sound/sdca_function.h>
#include <sound/sdca_interrupts.h>
+#include <sound/sdca_ump.h>
#include <sound/soc-component.h>
#include <sound/soc.h>
@@ -245,6 +247,29 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
return irqret;
}
+static irqreturn_t fdl_owner_handler(int irq, void *data)
+{
+ struct sdca_interrupt *interrupt = data;
+ struct device *dev = interrupt->dev;
+ irqreturn_t irqret = IRQ_NONE;
+ int ret;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to resume for fdl: %d\n", ret);
+ goto error;
+ }
+
+ ret = sdca_fdl_process(interrupt);
+ if (ret)
+ goto error;
+
+ irqret = IRQ_HANDLED;
+error:
+ pm_runtime_put(dev);
+ return irqret;
+}
+
static int sdca_irq_request_locked(struct device *dev,
struct sdca_interrupt_info *info,
int sdca_irq, const char *name,
@@ -423,6 +448,15 @@ int sdca_irq_populate(struct sdca_function_data *function,
if (control->sel == SDCA_CTL_GE_DETECTED_MODE)
handler = detected_mode_handler;
break;
+ case SDCA_ENTITY_TYPE_XU:
+ if (control->sel == SDCA_CTL_XU_FDL_CURRENTOWNER) {
+ ret = sdca_fdl_alloc_state(interrupt);
+ if (ret)
+ return ret;
+
+ handler = fdl_owner_handler;
+ }
+ break;
default:
break;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 16/19] ASoC: SDCA: Add completion for FDL start and stop
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (14 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 15/19] ASoC: SDCA: Add FDL-specific IRQ processing Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 17/19] ASoC: SDCA: Add UMP timeout handling for FDL Charles Keepax
` (4 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Add some completions and a helper function to allow other parts of the
system to wait for FDL to complete. The sdca_fdl_sync() function will
wait until it completes a full time out without a new FDL request
happening, this ensures that even parts requiring multiple rounds of FDL
should be fully downloaded before the driver boot continues.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Changes since v2:
- Reduce number of FDL retries
include/sound/sdca_fdl.h | 10 ++++++
sound/soc/sdca/sdca_fdl.c | 75 +++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+)
diff --git a/include/sound/sdca_fdl.h b/include/sound/sdca_fdl.h
index 8b025aff4a0cd..4ea000d6acef4 100644
--- a/include/sound/sdca_fdl.h
+++ b/include/sound/sdca_fdl.h
@@ -10,18 +10,26 @@
#ifndef __SDCA_FDL_H__
#define __SDCA_FDL_H__
+#include <linux/completion.h>
+
struct device;
struct regmap;
struct sdca_fdl_set;
struct sdca_function_data;
struct sdca_interrupt;
+struct sdca_interrupt_info;
/**
* struct fdl_state - FDL state structure to keep data between interrupts
+ * @begin: Completion indicating the start of an FDL download cycle.
+ * @done: Completion indicating the end of an FDL download cycle.
* @set: Pointer to the FDL set currently being downloaded.
* @file_index: Index of the current file being processed.
*/
struct fdl_state {
+ struct completion begin;
+ struct completion done;
+
struct sdca_fdl_set *set;
int file_index;
};
@@ -51,6 +59,8 @@ struct fdl_state {
int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt);
int sdca_fdl_process(struct sdca_interrupt *interrupt);
+int sdca_fdl_sync(struct device *dev, struct sdca_function_data *function,
+ struct sdca_interrupt_info *info);
int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
struct regmap *regmap);
diff --git a/sound/soc/sdca/sdca_fdl.c b/sound/soc/sdca/sdca_fdl.c
index 8a15c6300556c..39298314f69c9 100644
--- a/sound/soc/sdca/sdca_fdl.c
+++ b/sound/soc/sdca/sdca_fdl.c
@@ -14,6 +14,7 @@
#include <linux/firmware.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/sprintf.h>
#include <linux/soundwire/sdw.h>
@@ -63,6 +64,71 @@ int sdca_reset_function(struct device *dev, struct sdca_function_data *function,
}
EXPORT_SYMBOL_NS(sdca_reset_function, "SND_SOC_SDCA");
+/**
+ * sdca_fdl_sync - wait for a function to finish FDL
+ * @dev: Device pointer for error messages.
+ * @function: Pointer to the SDCA Function.
+ * @info: Pointer to the SDCA interrupt info for this device.
+ *
+ * Return: Zero on success or a negative error code.
+ */
+int sdca_fdl_sync(struct device *dev, struct sdca_function_data *function,
+ struct sdca_interrupt_info *info)
+{
+ static const int fdl_retries = 6;
+ unsigned long begin_timeout = msecs_to_jiffies(100);
+ unsigned long done_timeout = msecs_to_jiffies(4000);
+ int nfdl;
+ int i, j;
+
+ for (i = 0; i < fdl_retries; i++) {
+ nfdl = 0;
+
+ for (j = 0; j < SDCA_MAX_INTERRUPTS; j++) {
+ struct sdca_interrupt *interrupt = &info->irqs[j];
+ struct fdl_state *fdl_state;
+ unsigned long time;
+
+ if (interrupt->function != function ||
+ !interrupt->entity || !interrupt->control ||
+ interrupt->entity->type != SDCA_ENTITY_TYPE_XU ||
+ interrupt->control->sel != SDCA_CTL_XU_FDL_CURRENTOWNER)
+ continue;
+
+ fdl_state = interrupt->priv;
+ nfdl++;
+
+ /*
+ * Looking for timeout without any new FDL requests
+ * to imply the device has completed initial
+ * firmware setup. Alas the specification doesn't
+ * have any mechanism to detect this.
+ */
+ time = wait_for_completion_timeout(&fdl_state->begin,
+ begin_timeout);
+ if (!time) {
+ dev_dbg(dev, "no new FDL starts\n");
+ nfdl--;
+ continue;
+ }
+
+ time = wait_for_completion_timeout(&fdl_state->done,
+ done_timeout);
+ if (!time) {
+ dev_err(dev, "timed out waiting for FDL to complete\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ if (!nfdl)
+ return 0;
+ }
+
+ dev_err(dev, "too many FDL requests\n");
+ return -ETIMEDOUT;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_fdl_sync, "SND_SOC_SDCA");
+
static char *fdl_get_sku_filename(struct device *dev,
struct sdca_fdl_file *fdl_file)
{
@@ -230,6 +296,9 @@ static void fdl_end(struct sdca_interrupt *interrupt)
fdl_state->set = NULL;
+ pm_runtime_put(interrupt->dev);
+ complete(&fdl_state->done);
+
dev_dbg(interrupt->dev, "completed FDL process\n");
}
@@ -242,6 +311,9 @@ static int fdl_status_process(struct sdca_interrupt *interrupt, unsigned int sta
case SDCA_CTL_XU_FDLD_NEEDS_SET:
dev_dbg(interrupt->dev, "starting FDL process...\n");
+ pm_runtime_get(interrupt->dev);
+ complete(&fdl_state->begin);
+
fdl_state->file_index = 0;
fdl_state->set = fdl_get_set(interrupt);
fallthrough;
@@ -369,6 +441,9 @@ int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt)
if (!fdl_state)
return -ENOMEM;
+ init_completion(&fdl_state->begin);
+ init_completion(&fdl_state->done);
+
interrupt->priv = fdl_state;
return 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 17/19] ASoC: SDCA: Add UMP timeout handling for FDL
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (15 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 16/19] ASoC: SDCA: Add completion for FDL start and stop Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 18/19] ASoC: SDCA: Add early IRQ handling Charles Keepax
` (3 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Several of the UMP transactions in the FDL process should timeout if the
device does not respond within a certain time, add handling into the UMP
helpers and the FDL code to handle this.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Changes since v2:
- Add mutex to lock between UMP timeout work and IRQ handlers
- Clean up UMP timeout work after FDL fails
include/sound/sdca_fdl.h | 7 ++++++
include/sound/sdca_ump.h | 5 ++++
sound/soc/sdca/sdca_fdl.c | 50 ++++++++++++++++++++++++++++++++++++++-
sound/soc/sdca/sdca_ump.c | 15 ++++++++++++
4 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/include/sound/sdca_fdl.h b/include/sound/sdca_fdl.h
index 4ea000d6acef4..f4ba809cb203a 100644
--- a/include/sound/sdca_fdl.h
+++ b/include/sound/sdca_fdl.h
@@ -11,6 +11,7 @@
#define __SDCA_FDL_H__
#include <linux/completion.h>
+#include <linux/workqueue.h>
struct device;
struct regmap;
@@ -23,13 +24,19 @@ struct sdca_interrupt_info;
* struct fdl_state - FDL state structure to keep data between interrupts
* @begin: Completion indicating the start of an FDL download cycle.
* @done: Completion indicating the end of an FDL download cycle.
+ * @timeout: Delayed work used for timing out UMP transactions.
+ * @lock: Mutex to protect between the timeout work and IRQ handlers.
+ * @interrupt: Pointer to the interrupt struct to which this FDL is attached.
* @set: Pointer to the FDL set currently being downloaded.
* @file_index: Index of the current file being processed.
*/
struct fdl_state {
struct completion begin;
struct completion done;
+ struct delayed_work timeout;
+ struct mutex lock;
+ struct sdca_interrupt *interrupt;
struct sdca_fdl_set *set;
int file_index;
};
diff --git a/include/sound/sdca_ump.h b/include/sound/sdca_ump.h
index b2363199d19aa..f54f9d48c64cb 100644
--- a/include/sound/sdca_ump.h
+++ b/include/sound/sdca_ump.h
@@ -15,6 +15,7 @@ struct sdca_control;
struct sdca_entity;
struct sdca_function_data;
struct snd_soc_component;
+struct delayed_work;
int sdca_ump_get_owner_host(struct device *dev,
struct regmap *function_regmap,
@@ -42,4 +43,8 @@ int sdca_ump_write_message(struct device *dev,
unsigned int length_sel,
void *msg, int msg_len);
+void sdca_ump_cancel_timeout(struct delayed_work *work);
+void sdca_ump_schedule_timeout(struct delayed_work *work,
+ unsigned int timeout_us);
+
#endif // __SDCA_UMP_H__
diff --git a/sound/soc/sdca/sdca_fdl.c b/sound/soc/sdca/sdca_fdl.c
index 39298314f69c9..cb79dc3131b82 100644
--- a/sound/soc/sdca/sdca_fdl.c
+++ b/sound/soc/sdca/sdca_fdl.c
@@ -116,7 +116,7 @@ int sdca_fdl_sync(struct device *dev, struct sdca_function_data *function,
done_timeout);
if (!time) {
dev_err(dev, "timed out waiting for FDL to complete\n");
- return -ETIMEDOUT;
+ goto error;
}
}
@@ -125,6 +125,25 @@ int sdca_fdl_sync(struct device *dev, struct sdca_function_data *function,
}
dev_err(dev, "too many FDL requests\n");
+
+error:
+ for (j = 0; j < SDCA_MAX_INTERRUPTS; j++) {
+ struct sdca_interrupt *interrupt = &info->irqs[j];
+ struct fdl_state *fdl_state;
+
+ if (interrupt->function != function ||
+ !interrupt->entity || !interrupt->control ||
+ interrupt->entity->type != SDCA_ENTITY_TYPE_XU ||
+ interrupt->control->sel != SDCA_CTL_XU_FDL_CURRENTOWNER)
+ continue;
+
+ disable_irq(interrupt->irq);
+
+ fdl_state = interrupt->priv;
+
+ sdca_ump_cancel_timeout(&fdl_state->timeout);
+ }
+
return -ETIMEDOUT;
}
EXPORT_SYMBOL_NS_GPL(sdca_fdl_sync, "SND_SOC_SDCA");
@@ -302,6 +321,21 @@ static void fdl_end(struct sdca_interrupt *interrupt)
dev_dbg(interrupt->dev, "completed FDL process\n");
}
+static void sdca_fdl_timeout_work(struct work_struct *work)
+{
+ struct fdl_state *fdl_state = container_of(work, struct fdl_state,
+ timeout.work);
+ struct sdca_interrupt *interrupt = fdl_state->interrupt;
+ struct device *dev = interrupt->dev;
+
+ dev_err(dev, "FDL transaction timed out\n");
+
+ guard(mutex)(&fdl_state->lock);
+
+ fdl_end(interrupt);
+ sdca_reset_function(dev, interrupt->function, interrupt->function_regmap);
+}
+
static int fdl_status_process(struct sdca_interrupt *interrupt, unsigned int status)
{
struct fdl_state *fdl_state = interrupt->priv;
@@ -364,15 +398,20 @@ int sdca_fdl_process(struct sdca_interrupt *interrupt)
{
struct device *dev = interrupt->dev;
struct sdca_entity_xu *xu = &interrupt->entity->xu;
+ struct fdl_state *fdl_state = interrupt->priv;
unsigned int reg, status;
int response, ret;
+ guard(mutex)(&fdl_state->lock);
+
ret = sdca_ump_get_owner_host(dev, interrupt->function_regmap,
interrupt->function, interrupt->entity,
interrupt->control);
if (ret)
goto reset_function;
+ sdca_ump_cancel_timeout(&fdl_state->timeout);
+
reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
SDCA_CTL_XU_FDL_STATUS, 0);
ret = regmap_read(interrupt->function_regmap, reg, &status);
@@ -415,7 +454,13 @@ int sdca_fdl_process(struct sdca_interrupt *interrupt)
case SDCA_XU_RESET_FUNCTION:
goto reset_function;
}
+ case SDCA_CTL_XU_FDLH_COMPLETE:
+ if (status & SDCA_CTL_XU_FDLD_REQ_ABORT ||
+ status == SDCA_CTL_XU_FDLD_COMPLETE)
+ return 0;
+ fallthrough;
default:
+ sdca_ump_schedule_timeout(&fdl_state->timeout, xu->max_delay);
return 0;
}
@@ -441,8 +486,11 @@ int sdca_fdl_alloc_state(struct sdca_interrupt *interrupt)
if (!fdl_state)
return -ENOMEM;
+ INIT_DELAYED_WORK(&fdl_state->timeout, sdca_fdl_timeout_work);
init_completion(&fdl_state->begin);
init_completion(&fdl_state->done);
+ mutex_init(&fdl_state->lock);
+ fdl_state->interrupt = interrupt;
interrupt->priv = fdl_state;
diff --git a/sound/soc/sdca/sdca_ump.c b/sound/soc/sdca/sdca_ump.c
index 5dcad2f7ea05b..8aba3ff168728 100644
--- a/sound/soc/sdca/sdca_ump.c
+++ b/sound/soc/sdca/sdca_ump.c
@@ -245,3 +245,18 @@ int sdca_ump_write_message(struct device *dev,
return 0;
}
EXPORT_SYMBOL_NS_GPL(sdca_ump_write_message, "SND_SOC_SDCA");
+
+void sdca_ump_cancel_timeout(struct delayed_work *work)
+{
+ cancel_delayed_work_sync(work);
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_cancel_timeout, "SND_SOC_SDCA");
+
+void sdca_ump_schedule_timeout(struct delayed_work *work, unsigned int timeout_us)
+{
+ if (!timeout_us)
+ return;
+
+ queue_delayed_work(system_wq, work, usecs_to_jiffies(timeout_us));
+}
+EXPORT_SYMBOL_NS_GPL(sdca_ump_schedule_timeout, "SND_SOC_SDCA");
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 18/19] ASoC: SDCA: Add early IRQ handling
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (16 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 17/19] ASoC: SDCA: Add UMP timeout handling for FDL Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-20 15:55 ` [PATCH v3 RESEND 19/19] ASoC: SDCA: Add HID button IRQ Charles Keepax
` (2 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Some IRQs (FDL) require processing before the primary soundcard is
brought up, as the downloaded files could be firmware required for
operation of the audio functions of the device. Add a new helper
function which registers the required IRQs.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_interrupts.h | 3 ++
sound/soc/sdca/sdca_interrupts.c | 71 ++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/include/sound/sdca_interrupts.h b/include/sound/sdca_interrupts.h
index 3983f515349ad..8f13417d129ab 100644
--- a/include/sound/sdca_interrupts.h
+++ b/include/sound/sdca_interrupts.h
@@ -75,6 +75,9 @@ int sdca_irq_data_populate(struct device *dev, struct regmap *function_regmap,
struct sdca_entity *entity,
struct sdca_control *control,
struct sdca_interrupt *interrupt);
+int sdca_irq_populate_early(struct device *dev, struct regmap *function_regmap,
+ struct sdca_function_data *function,
+ struct sdca_interrupt_info *info);
int sdca_irq_populate(struct sdca_function_data *function,
struct snd_soc_component *component,
struct sdca_interrupt_info *info);
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 3a3b966b5782b..51342b8aacae9 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -396,6 +396,77 @@ static struct sdca_interrupt *get_interrupt_data(struct device *dev, int irq,
return &info->irqs[irq];
}
+/**
+ * sdca_irq_populate_early - process pre-audio card IRQ registrations
+ * @dev: Device pointer for SDCA Function.
+ * @regmap: Regmap pointer for the SDCA Function.
+ * @function: Pointer to the SDCA Function.
+ * @info: Pointer to the SDCA interrupt info for this device.
+ *
+ * This is intended to be used as part of the Function boot process. It
+ * can be called before the soundcard is registered (ie. doesn't depend
+ * on component) and will register the FDL interrupts.
+ *
+ * Return: Zero on success, and a negative error code on failure.
+ */
+int sdca_irq_populate_early(struct device *dev, struct regmap *regmap,
+ struct sdca_function_data *function,
+ struct sdca_interrupt_info *info)
+{
+ int i, j;
+
+ guard(mutex)(&info->irq_lock);
+
+ 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];
+ int irq = control->interrupt_position;
+ struct sdca_interrupt *interrupt;
+ int ret;
+
+ interrupt = get_interrupt_data(dev, irq, info);
+ if (IS_ERR(interrupt))
+ return PTR_ERR(interrupt);
+ else if (!interrupt)
+ continue;
+
+ switch (entity->type) {
+ case SDCA_ENTITY_TYPE_XU:
+ if (control->sel != SDCA_CTL_XU_FDL_CURRENTOWNER)
+ break;
+
+ ret = sdca_irq_data_populate(dev, regmap, NULL,
+ function, entity,
+ control, interrupt);
+ if (ret)
+ return ret;
+
+ ret = sdca_fdl_alloc_state(interrupt);
+ if (ret)
+ return ret;
+
+ ret = sdca_irq_request_locked(dev, info, irq,
+ interrupt->name,
+ fdl_owner_handler,
+ interrupt);
+ if (ret) {
+ dev_err(dev, "failed to request irq %s: %d\n",
+ interrupt->name, ret);
+ return ret;
+ }
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(sdca_irq_populate_early, "SND_SOC_SDCA");
+
/**
* sdca_irq_populate - Request all the individual IRQs for an SDCA Function
* @function: Pointer to the SDCA Function.
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 RESEND 19/19] ASoC: SDCA: Add HID button IRQ
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (17 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 18/19] ASoC: SDCA: Add early IRQ handling Charles Keepax
@ 2025-10-20 15:55 ` Charles Keepax
2025-10-27 14:43 ` [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Pierre-Louis Bossart
2025-10-29 22:02 ` Mark Brown
20 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2025-10-20 15:55 UTC (permalink / raw)
To: broonie
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
Now full support for the UMP buffers is available, it is possible to
read the SDCA HID descriptors from the device and pass them to
user-space. Add a helper function to process HID events from an SDCA
device.
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No changes since v2.
include/sound/sdca_hid.h | 13 +++++++--
sound/soc/sdca/sdca_hid.c | 46 ++++++++++++++++++++++++++++++++
sound/soc/sdca/sdca_interrupts.c | 28 +++++++++++++++++++
3 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/include/sound/sdca_hid.h b/include/sound/sdca_hid.h
index 3a155835e035e..18bebbe428c9f 100644
--- a/include/sound/sdca_hid.h
+++ b/include/sound/sdca_hid.h
@@ -8,13 +8,17 @@
#ifndef __SDCA_HID_H__
#define __SDCA_HID_H__
-#include <linux/types.h>
-#include <linux/hid.h>
+struct device;
+struct sdw_slave;
+
+struct sdca_entity;
+struct sdca_interrupt;
#if IS_ENABLED(CONFIG_SND_SOC_SDCA_HID)
int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
struct sdca_entity *entity);
+int sdca_hid_process_report(struct sdca_interrupt *interrupt);
#else
@@ -24,6 +28,11 @@ static inline int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
return 0;
}
+static inline int sdca_hid_process_report(struct sdca_interrupt *interrupt)
+{
+ return 0;
+}
+
#endif
#endif /* __SDCA_HID_H__ */
diff --git a/sound/soc/sdca/sdca_hid.c b/sound/soc/sdca/sdca_hid.c
index 53dad1a524d4b..ad53207b0d62b 100644
--- a/sound/soc/sdca/sdca_hid.c
+++ b/sound/soc/sdca/sdca_hid.c
@@ -10,6 +10,7 @@
#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/dev_printk.h>
+#include <linux/hid.h>
#include <linux/module.h>
#include <linux/property.h>
#include <linux/soundwire/sdw.h>
@@ -17,6 +18,8 @@
#include <sound/sdca.h>
#include <sound/sdca_function.h>
#include <sound/sdca_hid.h>
+#include <sound/sdca_interrupts.h>
+#include <sound/sdca_ump.h>
static int sdwhid_parse(struct hid_device *hid)
{
@@ -121,5 +124,48 @@ int sdca_add_hid_device(struct device *dev, struct sdw_slave *sdw,
}
EXPORT_SYMBOL_NS(sdca_add_hid_device, "SND_SOC_SDCA");
+/**
+ * sdca_hid_process_report - read a HID event from the device and report
+ * @interrupt: Pointer to the SDCA interrupt information structure.
+ *
+ * Return: Zero on success, and a negative error code on failure.
+ */
+int sdca_hid_process_report(struct sdca_interrupt *interrupt)
+{
+ struct device *dev = interrupt->dev;
+ struct hid_device *hid = interrupt->entity->hide.hid;
+ void *val __free(kfree) = NULL;
+ int len, ret;
+
+ ret = sdca_ump_get_owner_host(dev, interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ interrupt->control);
+ if (ret)
+ return ret;
+
+ len = sdca_ump_read_message(dev, interrupt->device_regmap,
+ interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ SDCA_CTL_HIDE_HIDTX_MESSAGEOFFSET,
+ SDCA_CTL_HIDE_HIDTX_MESSAGELENGTH, &val);
+ if (len < 0)
+ return len;
+
+ ret = sdca_ump_set_owner_device(dev, interrupt->function_regmap,
+ interrupt->function, interrupt->entity,
+ interrupt->control);
+ if (ret)
+ return ret;
+
+ ret = hid_input_report(hid, HID_INPUT_REPORT, val, len, true);
+ if (ret < 0) {
+ dev_err(dev, "failed to report hid event: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS(sdca_hid_process_report, "SND_SOC_SDCA");
+
MODULE_LICENSE("Dual BSD/GPL");
MODULE_DESCRIPTION("SDCA HID library");
diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
index 51342b8aacae9..5176460416bbe 100644
--- a/sound/soc/sdca/sdca_interrupts.c
+++ b/sound/soc/sdca/sdca_interrupts.c
@@ -20,6 +20,7 @@
#include <sound/sdca.h>
#include <sound/sdca_fdl.h>
#include <sound/sdca_function.h>
+#include <sound/sdca_hid.h>
#include <sound/sdca_interrupts.h>
#include <sound/sdca_ump.h>
#include <sound/soc-component.h>
@@ -247,6 +248,29 @@ static irqreturn_t detected_mode_handler(int irq, void *data)
return irqret;
}
+static irqreturn_t hid_handler(int irq, void *data)
+{
+ struct sdca_interrupt *interrupt = data;
+ struct device *dev = interrupt->dev;
+ irqreturn_t irqret = IRQ_NONE;
+ int ret;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to resume for hid: %d\n", ret);
+ goto error;
+ }
+
+ ret = sdca_hid_process_report(interrupt);
+ if (ret)
+ goto error;
+
+ irqret = IRQ_HANDLED;
+error:
+ pm_runtime_put(dev);
+ return irqret;
+}
+
static irqreturn_t fdl_owner_handler(int irq, void *data)
{
struct sdca_interrupt *interrupt = data;
@@ -528,6 +552,10 @@ int sdca_irq_populate(struct sdca_function_data *function,
handler = fdl_owner_handler;
}
break;
+ case SDCA_ENTITY_TYPE_HIDE:
+ if (control->sel == SDCA_CTL_HIDE_HIDTX_CURRENTOWNER)
+ handler = hid_handler;
+ break;
default:
break;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (18 preceding siblings ...)
2025-10-20 15:55 ` [PATCH v3 RESEND 19/19] ASoC: SDCA: Add HID button IRQ Charles Keepax
@ 2025-10-27 14:43 ` Pierre-Louis Bossart
2025-10-29 22:02 ` Mark Brown
20 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2025-10-27 14:43 UTC (permalink / raw)
To: Charles Keepax, broonie
Cc: yung-chuan.liao, peter.ujfalusi, shumingf, lgirdwood, linux-sound,
patches
On 10/20/25 17:54, Charles Keepax wrote:
> Next installment of the SDCA changes, hopefully the next series after
> this should be the full class driver. It is worth noting this series has
> a build dependency on a patch working its way through the PM/ACPI tree:
>
> commit ac46f5b6c661 ("ACPICA: Add SoundWire File Table (SWFT)
> signature")
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
>
> But we can probably worry about that later, as normally there is a
> reasonable amount of review on these SDCA series'.
>
> This series broadly breaks down into 3 chunks, first there are several
> changes to remove the assumption that the struct device used for SDCA
> purposes represents the SoundWire slave. This is because the SDCA class
> driver will be made of an auxiliary driver for each SDCA Function, thus
> the SoundWire slave will be on the parent device for each individual
> driver. Then there are patches to add support for UMP/FDL. And then
> finally since the rest of the HID support is there and UMP was the last
> missing part required a small patch to add a function to allow reporting
> of HID events from SDCA devices.
I added minor comments on one FDL patch, but no reason to delay this series further, firmware download is a must-have for newer devices.
For the series:
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Nice work Charles!
> Changes since v2:
> - Add mutex to lock between UMP timeout work and IRQ handlers
> - Clean up UMP timeout work after FDL fails
> - Use IRQ number rather than a boolean to track used IRQs
> - Reduce FDL retries to 6
> - Check disk size of firmwares
> - Reset on more FDL failure paths
> - Reorder volatile control check by entity
>
> Changes since v1:
> - Add timeout for UMP buffer transfers
> - Add function reset
> - Parse XU properties from DisCo
> - Rename entity_xu library to FDL
> - Add a limit to the number of times it will try the FDL process
> - Rename soundwire device pointers to sdev to distinguish from Function
> devices pointers
>
> Charles Keepax (16):
> ASoC: SDCA: Rename SoundWire struct device variables
> regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave
> ASoC: SDCA: Add manual PM runtime gets to IRQ handlers
> ASoC: SDCA: Pass SoundWire slave to HID
> ASoC: SDCA: Pass device register map from IRQ alloc to handlers
> ASoC: SDCA: Update externally_requested flag to cover all requests
> ASoC: SDCA: Factor out a helper to find SDCA IRQ data
> ASoC: SDCA: Rely less on the ASoC component in IRQ handling
> ASoC: SDCA: Force some SDCA Controls to be volatile
> ASoC: SDCA: Parse XU Entity properties
> ASoC: SDCA: Parse Function Reset max delay
> ASoC: SDCA: Add UMP buffer helper functions
> ASoC: SDCA: Add completion for FDL start and stop
> ASoC: SDCA: Add UMP timeout handling for FDL
> ASoC: SDCA: Add early IRQ handling
> ASoC: SDCA: Add HID button IRQ
>
> Maciej Strozek (3):
> ASoC: SDCA: Add SDCA FDL data parsing
> ASoC: SDCA: Add FDL library for XU entities
> ASoC: SDCA: Add FDL-specific IRQ processing
>
> drivers/base/regmap/regmap-sdw-mbq.c | 23 +-
> include/linux/regmap.h | 21 +-
> include/sound/sdca.h | 5 +
> include/sound/sdca_fdl.h | 75 ++++
> include/sound/sdca_function.h | 119 ++++++-
> include/sound/sdca_hid.h | 21 +-
> include/sound/sdca_interrupts.h | 19 +-
> include/sound/sdca_ump.h | 50 +++
> sound/soc/codecs/rt722-sdca-sdw.c | 4 +-
> sound/soc/sdca/Kconfig | 8 +
> sound/soc/sdca/Makefile | 4 +-
> sound/soc/sdca/sdca_device.c | 20 ++
> sound/soc/sdca/sdca_fdl.c | 499 +++++++++++++++++++++++++++
> sound/soc/sdca/sdca_functions.c | 212 +++++++++++-
> sound/soc/sdca/sdca_hid.c | 58 +++-
> sound/soc/sdca/sdca_interrupts.c | 271 ++++++++++++---
> sound/soc/sdca/sdca_regmap.c | 9 +-
> sound/soc/sdca/sdca_ump.c | 262 ++++++++++++++
> 18 files changed, 1573 insertions(+), 107 deletions(-)
> create mode 100644 include/sound/sdca_fdl.h
> create mode 100644 include/sound/sdca_ump.h
> create mode 100644 sound/soc/sdca/sdca_fdl.c
> create mode 100644 sound/soc/sdca/sdca_ump.c
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support
2025-10-20 15:54 [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Charles Keepax
` (19 preceding siblings ...)
2025-10-27 14:43 ` [PATCH v3 RESEND 00/19] Add SDCA UMP/FDL support Pierre-Louis Bossart
@ 2025-10-29 22:02 ` Mark Brown
20 siblings, 0 replies; 24+ messages in thread
From: Mark Brown @ 2025-10-29 22:02 UTC (permalink / raw)
To: Charles Keepax
Cc: yung-chuan.liao, pierre-louis.bossart, peter.ujfalusi, shumingf,
lgirdwood, linux-sound, patches
On Mon, 20 Oct 2025 16:54:53 +0100, Charles Keepax wrote:
> Next installment of the SDCA changes, hopefully the next series after
> this should be the full class driver. It is worth noting this series has
> a build dependency on a patch working its way through the PM/ACPI tree:
>
> commit ac46f5b6c661 ("ACPICA: Add SoundWire File Table (SWFT)
> signature")
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/19] ASoC: SDCA: Rename SoundWire struct device variables
commit: 715159314dfafee66e6deb50b4e3431539a919d8
[02/19] regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave
commit: 013a3a66f25af3fb614f45df43983657514944c4
[03/19] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers
commit: 907364ea3db47530751add6d2d62122ca17329cb
[04/19] ASoC: SDCA: Pass SoundWire slave to HID
commit: 7159816707dc7040fe3ac4fa3d7ac3d173bd772a
[05/19] ASoC: SDCA: Pass device register map from IRQ alloc to handlers
commit: 390c05f47d0749b24db65586482308c5fd680fe5
[06/19] ASoC: SDCA: Update externally_requested flag to cover all requests
commit: 56bbda23d4bece7ce998666118a068e4f71d59fb
[07/19] ASoC: SDCA: Factor out a helper to find SDCA IRQ data
commit: 8d557cc4867f2008f440c54b4423464301a1ef4b
[08/19] ASoC: SDCA: Rely less on the ASoC component in IRQ handling
commit: dfe7c3401ed3d3bd8e61be8d6d452896513eb52e
[09/19] ASoC: SDCA: Force some SDCA Controls to be volatile
commit: c7b6c6b60594fd1efe35c61bc6a2176b25263ccc
[10/19] ASoC: SDCA: Parse XU Entity properties
commit: 0a5e9769d088bd1d8faf01207210911b9341b62c
[11/19] ASoC: SDCA: Parse Function Reset max delay
commit: 7b6be935e7eff06025e18cea4c6620194450abe2
[12/19] ASoC: SDCA: Add UMP buffer helper functions
commit: daab108504be73182c16a72b9cfe47ac3b1928ca
[13/19] ASoC: SDCA: Add SDCA FDL data parsing
commit: c4d096c3ca425562192a3626c30e82651d0f2c1c
[14/19] ASoC: SDCA: Add FDL library for XU entities
commit: 71f7990a34cdb11f82d3cbbcddaca77a55635466
[15/19] ASoC: SDCA: Add FDL-specific IRQ processing
commit: aeaf27ec6571527e750eed84bb3865a0664ae316
[16/19] ASoC: SDCA: Add completion for FDL start and stop
commit: 0723affa1bee50c3bd7ca00e00dee07fcef224b8
[17/19] ASoC: SDCA: Add UMP timeout handling for FDL
commit: e92e25f777483b7cc3e170214cc84337d7a415cf
[18/19] ASoC: SDCA: Add early IRQ handling
commit: 12aa3160c10a3179c73c4f99a2d5aec0fd907d0c
[19/19] ASoC: SDCA: Add HID button IRQ
commit: ef042df96d0e1089764f39ede61bc8f140a4be00
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 24+ messages in thread