[PATCH v6 0/2] Read _SUB from ACPI to be able to identify firmware
CS35L41 has a DSP which is able to run firmware, as well as a tuning file. Different systems may want to use different firmwares and tuning files, and some firmwares/tunings may not be compatible with other systems. To allow a system to select the correct fimware/tuning, we can read an _SUB from the ACPI. This _SUB can then be used to uniquely identify the system in the firmware/tuning file name.
Add a helper function which reads the _SUB, so this can be used by other parts in the future. Add support inside the CS35L41 ASoC driver to read this _SUB, and save it appropriately.
Changes since v5: - Clean up return codes - Refactor length calculation - Allow fallback to existing behaviour when bad ACPI is detected
Changes since v4: - Rename function
Changes since v3: - Fix 32 bit format string warning
Changes since v2: - Fix error in function prototype
Changes since v1: - Add length validation for SSID String - Rename API - Allocate memory inside API - Use ACPI_HANDLE macro instead of ACPI_COMPANION - Improve error handling
Stefan Binding (2): ACPI: utils: Add api to read _SUB from ACPI ASoC: cs35l41: Read System Name from ACPI _SUB to identify firmware
drivers/acpi/utils.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 6 ++++++ sound/soc/codecs/cs35l41.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+)
Add a wrapper function to read the _SUB string from ACPI.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com --- drivers/acpi/utils.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 6 ++++++ 2 files changed, 44 insertions(+)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 3a9773a09e19..5a7b8065e77f 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -291,6 +291,44 @@ int acpi_get_local_address(acpi_handle handle, u32 *addr) } EXPORT_SYMBOL(acpi_get_local_address);
+#define ACPI_MAX_SUB_BUF_SIZE 9 + +const char *acpi_get_subsystem_id(acpi_handle handle) +{ + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *obj; + acpi_status status; + const char *sub; + size_t len; + + status = acpi_evaluate_object(handle, METHOD_NAME__SUB, NULL, &buffer); + if (ACPI_FAILURE(status)) { + acpi_handle_debug(handle, "Reading ACPI _SUB failed: %#x\n", status); + return ERR_PTR(-ENODATA); + } + + obj = buffer.pointer; + if (obj->type == ACPI_TYPE_STRING) { + len = strlen(obj->string.pointer); + if (len < ACPI_MAX_SUB_BUF_SIZE && len > 0) { + sub = kstrdup(obj->string.pointer, GFP_KERNEL); + if (!sub) + sub = ERR_PTR(-ENOMEM); + } else { + acpi_handle_err(handle, "ACPI _SUB Length %zu is Invalid\n", len); + sub = ERR_PTR(-ENODATA); + } + } else { + acpi_handle_warn(handle, "Warning ACPI _SUB did not return a string\n"); + sub = ERR_PTR(-ENODATA); + } + + acpi_os_free(buffer.pointer); + + return sub; +} +EXPORT_SYMBOL_GPL(acpi_get_subsystem_id); + acpi_status acpi_evaluate_reference(acpi_handle handle, acpi_string pathname, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 2a0e95336094..447898685e09 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -762,6 +762,7 @@ static inline u64 acpi_arch_get_root_pointer(void) #endif
int acpi_get_local_address(acpi_handle handle, u32 *addr); +const char *acpi_get_subsystem_id(acpi_handle handle);
#else /* !CONFIG_ACPI */
@@ -1023,6 +1024,11 @@ static inline int acpi_get_local_address(acpi_handle handle, u32 *addr) return -ENODEV; }
+static inline const char *acpi_get_subsystem_id(acpi_handle handle) +{ + return ERR_PTR(-ENODEV); +} + static inline int acpi_register_wakeup_handler(int wake_irq, bool (*wakeup)(void *context), void *context) {
On Thu, Jul 7, 2022 at 5:11 PM Stefan Binding sbinding@opensource.cirrus.com wrote:
Add a wrapper function to read the _SUB string from ACPI.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com
In case you want to push this through ASoC
Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
Otherwise, I need a maintainer's ACK to apply the second patch.
drivers/acpi/utils.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 6 ++++++ 2 files changed, 44 insertions(+)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 3a9773a09e19..5a7b8065e77f 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -291,6 +291,44 @@ int acpi_get_local_address(acpi_handle handle, u32 *addr) } EXPORT_SYMBOL(acpi_get_local_address);
+#define ACPI_MAX_SUB_BUF_SIZE 9
+const char *acpi_get_subsystem_id(acpi_handle handle) +{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };union acpi_object *obj;acpi_status status;const char *sub;size_t len;status = acpi_evaluate_object(handle, METHOD_NAME__SUB, NULL, &buffer);if (ACPI_FAILURE(status)) {acpi_handle_debug(handle, "Reading ACPI _SUB failed: %#x\n", status);return ERR_PTR(-ENODATA);}obj = buffer.pointer;if (obj->type == ACPI_TYPE_STRING) {len = strlen(obj->string.pointer);if (len < ACPI_MAX_SUB_BUF_SIZE && len > 0) {sub = kstrdup(obj->string.pointer, GFP_KERNEL);if (!sub)sub = ERR_PTR(-ENOMEM);} else {acpi_handle_err(handle, "ACPI _SUB Length %zu is Invalid\n", len);sub = ERR_PTR(-ENODATA);}} else {acpi_handle_warn(handle, "Warning ACPI _SUB did not return a string\n");sub = ERR_PTR(-ENODATA);}acpi_os_free(buffer.pointer);return sub;+} +EXPORT_SYMBOL_GPL(acpi_get_subsystem_id);
acpi_status acpi_evaluate_reference(acpi_handle handle, acpi_string pathname, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 2a0e95336094..447898685e09 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -762,6 +762,7 @@ static inline u64 acpi_arch_get_root_pointer(void) #endif
int acpi_get_local_address(acpi_handle handle, u32 *addr); +const char *acpi_get_subsystem_id(acpi_handle handle);
#else /* !CONFIG_ACPI */
@@ -1023,6 +1024,11 @@ static inline int acpi_get_local_address(acpi_handle handle, u32 *addr) return -ENODEV; }
+static inline const char *acpi_get_subsystem_id(acpi_handle handle) +{
return ERR_PTR(-ENODEV);+}
static inline int acpi_register_wakeup_handler(int wake_irq, bool (*wakeup)(void *context), void *context) { -- 2.25.1
When loading firmware, wm_adsp uses a number of parameters to determine the path of the firmware and tuning files to load. One of these parameters is system_name. Add support in cs35l41 to read this system name from the ACPI _SUB ID in order to uniquely identify the firmware and tuning mapped to a particular system.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com --- sound/soc/codecs/cs35l41.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c index 8766e19d85f1..c223d83e02cf 100644 --- a/sound/soc/codecs/cs35l41.c +++ b/sound/soc/codecs/cs35l41.c @@ -6,6 +6,7 @@ // // Author: David Rhodes david.rhodes@cirrus.com
+#include <linux/acpi.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/init.h> @@ -1142,6 +1143,30 @@ static int cs35l41_dsp_init(struct cs35l41_private *cs35l41) return ret; }
+static int cs35l41_acpi_get_name(struct cs35l41_private *cs35l41) +{ + acpi_handle handle = ACPI_HANDLE(cs35l41->dev); + const char *sub; + + /* If there is no ACPI_HANDLE, there is no ACPI for this system, return 0 */ + if (!handle) + return 0; + + sub = acpi_get_subsystem_id(handle); + if (IS_ERR(sub)) { + /* If bad ACPI, return 0 and fallback to legacy firmware path, otherwise fail */ + if (PTR_ERR(sub) == -ENODATA) + return 0; + else + return PTR_ERR(sub); + } + + cs35l41->dsp.system_name = sub; + dev_dbg(cs35l41->dev, "Subsystem ID: %s\n", cs35l41->dsp.system_name); + + return 0; +} + int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg *hw_cfg) { u32 regid, reg_revid, i, mtl_revid, int_status, chipid_match; @@ -1270,6 +1295,10 @@ int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg * goto err; }
+ ret = cs35l41_acpi_get_name(cs35l41); + if (ret < 0) + goto err; + ret = cs35l41_dsp_init(cs35l41); if (ret < 0) goto err; @@ -1316,6 +1345,7 @@ void cs35l41_remove(struct cs35l41_private *cs35l41) pm_runtime_disable(cs35l41->dev);
regmap_write(cs35l41->regmap, CS35L41_IRQ1_MASK1, 0xFFFFFFFF); + kfree(cs35l41->dsp.system_name); wm_adsp2_remove(&cs35l41->dsp); cs35l41_safe_reset(cs35l41->regmap, cs35l41->hw_cfg.bst_type);
On Thu, 7 Jul 2022 16:10:35 +0100, Stefan Binding wrote:
CS35L41 has a DSP which is able to run firmware, as well as a tuning file. Different systems may want to use different firmwares and tuning files, and some firmwares/tunings may not be compatible with other systems. To allow a system to select the correct fimware/tuning, we can read an _SUB from the ACPI. This _SUB can then be used to uniquely identify the system in the firmware/tuning file name.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ACPI: utils: Add api to read _SUB from ACPI commit: 93064e15c8a3a8394319a11b8037666e4b7d653d [2/2] ASoC: cs35l41: Read System Name from ACPI _SUB to identify firmware commit: c1ad138822a1be95a7a7b122521c2415583a0c26
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
participants (3)
-
Mark Brown -
Rafael J. Wysocki -
Stefan Binding